From add2ce5027a6d0b080ce8de45107371eb92b7a64 Mon Sep 17 00:00:00 2001 From: Philip Craig Date: Sun, 12 Nov 2023 13:20:42 +1000 Subject: [PATCH] read/coff: SymbolKind::Common detection was too broad Common symbols should be IMAGE_SYM_CLASS_EXTERNAL. We were also matching on IMAGE_SYM_CLASS_SECTION, which can occur for section symbols in import libraries where the section is defined in another archive member. --- .../examples/testfiles/coff/import_msvc.lib.objdump | 12 ++++++------ src/read/coff/symbol.rs | 12 +++++++----- 2 files changed, 13 insertions(+), 11 deletions(-) diff --git a/crates/examples/testfiles/coff/import_msvc.lib.objdump b/crates/examples/testfiles/coff/import_msvc.lib.objdump index 2d62f547..49c19c0a 100644 --- a/crates/examples/testfiles/coff/import_msvc.lib.objdump +++ b/crates/examples/testfiles/coff/import_msvc.lib.objdump @@ -89,8 +89,8 @@ Symbols 1: Symbol { name: "__IMPORT_DESCRIPTOR_test_x64", address: 0, size: 0, kind: Data, section: Section(SectionIndex(2)), scope: Linkage, weak: false, flags: None } 2: Symbol { name: ".idata$2", address: 0, size: 0, kind: Section, section: Section(SectionIndex(2)), scope: Compilation, weak: false, flags: None } 3: Symbol { name: ".idata$6", address: 0, size: 0, kind: Data, section: Section(SectionIndex(3)), scope: Compilation, weak: false, flags: None } -4: Symbol { name: ".idata$4", address: 0, size: 0, kind: Section, section: Common, scope: Compilation, weak: false, flags: None } -5: Symbol { name: ".idata$5", address: 0, size: 0, kind: Section, section: Common, scope: Compilation, weak: false, flags: None } +4: Symbol { name: ".idata$4", address: 0, size: 0, kind: Section, section: Unknown, scope: Compilation, weak: false, flags: None } +5: Symbol { name: ".idata$5", address: 0, size: 0, kind: Section, section: Unknown, scope: Compilation, weak: false, flags: None } 6: Symbol { name: "__NULL_IMPORT_DESCRIPTOR", address: 0, size: 0, kind: Data, section: Undefined, scope: Linkage, weak: false, flags: None } 7: Symbol { name: "\u{7f}test_x64_NULL_THUNK_DATA", address: 0, size: 0, kind: Data, section: Undefined, scope: Linkage, weak: false, flags: None } @@ -166,8 +166,8 @@ Symbols 1: Symbol { name: "__IMPORT_DESCRIPTOR_test_x86", address: 0, size: 0, kind: Data, section: Section(SectionIndex(2)), scope: Linkage, weak: false, flags: None } 2: Symbol { name: ".idata$2", address: 0, size: 0, kind: Section, section: Section(SectionIndex(2)), scope: Compilation, weak: false, flags: None } 3: Symbol { name: ".idata$6", address: 0, size: 0, kind: Data, section: Section(SectionIndex(3)), scope: Compilation, weak: false, flags: None } -4: Symbol { name: ".idata$4", address: 0, size: 0, kind: Section, section: Common, scope: Compilation, weak: false, flags: None } -5: Symbol { name: ".idata$5", address: 0, size: 0, kind: Section, section: Common, scope: Compilation, weak: false, flags: None } +4: Symbol { name: ".idata$4", address: 0, size: 0, kind: Section, section: Unknown, scope: Compilation, weak: false, flags: None } +5: Symbol { name: ".idata$5", address: 0, size: 0, kind: Section, section: Unknown, scope: Compilation, weak: false, flags: None } 6: Symbol { name: "__NULL_IMPORT_DESCRIPTOR", address: 0, size: 0, kind: Data, section: Undefined, scope: Linkage, weak: false, flags: None } 7: Symbol { name: "\u{7f}test_x86_NULL_THUNK_DATA", address: 0, size: 0, kind: Data, section: Undefined, scope: Linkage, weak: false, flags: None } @@ -243,8 +243,8 @@ Symbols 1: Symbol { name: "__IMPORT_DESCRIPTOR_test_arm64ec", address: 0, size: 0, kind: Data, section: Section(SectionIndex(2)), scope: Linkage, weak: false, flags: None } 2: Symbol { name: ".idata$2", address: 0, size: 0, kind: Section, section: Section(SectionIndex(2)), scope: Compilation, weak: false, flags: None } 3: Symbol { name: ".idata$6", address: 0, size: 0, kind: Data, section: Section(SectionIndex(3)), scope: Compilation, weak: false, flags: None } -4: Symbol { name: ".idata$4", address: 0, size: 0, kind: Section, section: Common, scope: Compilation, weak: false, flags: None } -5: Symbol { name: ".idata$5", address: 0, size: 0, kind: Section, section: Common, scope: Compilation, weak: false, flags: None } +4: Symbol { name: ".idata$4", address: 0, size: 0, kind: Section, section: Unknown, scope: Compilation, weak: false, flags: None } +5: Symbol { name: ".idata$5", address: 0, size: 0, kind: Section, section: Unknown, scope: Compilation, weak: false, flags: None } 6: Symbol { name: "__NULL_IMPORT_DESCRIPTOR", address: 0, size: 0, kind: Data, section: Undefined, scope: Linkage, weak: false, flags: None } 7: Symbol { name: "\u{7f}test_arm64ec_NULL_THUNK_DATA", address: 0, size: 0, kind: Data, section: Undefined, scope: Linkage, weak: false, flags: None } diff --git a/src/read/coff/symbol.rs b/src/read/coff/symbol.rs index e95468d7..8e98ca6c 100644 --- a/src/read/coff/symbol.rs +++ b/src/read/coff/symbol.rs @@ -401,12 +401,14 @@ impl<'data, 'file, R: ReadRef<'data>, Coff: CoffHeader> ObjectSymbol<'data> fn section(&self) -> SymbolSection { match self.symbol.section_number() { pe::IMAGE_SYM_UNDEFINED => { - if self.symbol.storage_class() == pe::IMAGE_SYM_CLASS_EXTERNAL - && self.symbol.value() == 0 - { - SymbolSection::Undefined + if self.symbol.storage_class() == pe::IMAGE_SYM_CLASS_EXTERNAL { + if self.symbol.value() == 0 { + SymbolSection::Undefined + } else { + SymbolSection::Common + } } else { - SymbolSection::Common + SymbolSection::Unknown } } pe::IMAGE_SYM_ABSOLUTE => SymbolSection::Absolute,