From 11582abc270028b448a49ab97459bfacd958dd2a Mon Sep 17 00:00:00 2001 From: "thestig@chromium.org" Date: Thu, 20 Oct 2011 18:23:01 +0000 Subject: Fix some shadow variables, including one in file_id.cc that causes all files to generate the same hash. Add a test to make sure this doesn't happen again. Review URL: http://breakpad.appspot.com/316002 git-svn-id: http://google-breakpad.googlecode.com/svn/trunk@875 4c0a9323-5329-0410-9bdc-e9ce6186880e --- src/common/linux/file_id.cc | 18 +++---- src/common/linux/file_id_unittest.cc | 101 +++++++++++++++++++++++++++++++++++ 2 files changed, 108 insertions(+), 11 deletions(-) (limited to 'src/common/linux') diff --git a/src/common/linux/file_id.cc b/src/common/linux/file_id.cc index acd448a9..13f4f3ad 100644 --- a/src/common/linux/file_id.cc +++ b/src/common/linux/file_id.cc @@ -107,10 +107,10 @@ static void FindElfClassSection(const char *elf_base, const Shdr* section = NULL; for (int i = 0; i < elf_header->e_shnum; ++i) { if (sections[i].sh_type == section_type) { - const char* section_name = (char*)(elf_base + - string_section->sh_offset + - sections[i].sh_name); - if (!my_strncmp(section_name, section_name, name_len)) { + const char* current_section_name = (char*)(elf_base + + string_section->sh_offset + + sections[i].sh_name); + if (!my_strncmp(current_section_name, section_name, name_len)) { section = §ions[i]; break; } @@ -166,8 +166,7 @@ static bool FindElfSection(const void *elf_mapped_base, template static bool ElfClassBuildIDNoteIdentifier(const void *section, - uint8_t identifier[kMDGUIDSize]) -{ + uint8_t identifier[kMDGUIDSize]) { typedef typename ElfClass::Nhdr Nhdr; const Nhdr* note_header = reinterpret_cast(section); @@ -190,8 +189,7 @@ static bool ElfClassBuildIDNoteIdentifier(const void *section, // Attempt to locate a .note.gnu.build-id section in an ELF binary // and copy as many bytes of it as will fit into |identifier|. static bool FindElfBuildIDNote(const void *elf_mapped_base, - uint8_t identifier[kMDGUIDSize]) -{ + uint8_t identifier[kMDGUIDSize]) { void* note_section; int note_size, elfclass; if (!FindElfSection(elf_mapped_base, ".note.gnu.build-id", SHT_NOTE, @@ -213,7 +211,6 @@ static bool FindElfBuildIDNote(const void *elf_mapped_base, // a simple hash by XORing the first page worth of bytes into |identifier|. static bool HashElfTextSection(const void *elf_mapped_base, uint8_t identifier[kMDGUIDSize]) { - void* text_section; int text_size; if (!FindElfSection(elf_mapped_base, ".text", SHT_PROGBITS, @@ -235,8 +232,7 @@ static bool HashElfTextSection(const void *elf_mapped_base, // static bool FileID::ElfFileIdentifierFromMappedFile(void* base, - uint8_t identifier[kMDGUIDSize]) -{ + uint8_t identifier[kMDGUIDSize]) { // Look for a build id note first. if (FindElfBuildIDNote(base, identifier)) return true; diff --git a/src/common/linux/file_id_unittest.cc b/src/common/linux/file_id_unittest.cc index 24208983..7479a966 100644 --- a/src/common/linux/file_id_unittest.cc +++ b/src/common/linux/file_id_unittest.cc @@ -43,6 +43,18 @@ using google_breakpad::synth_elf::ELF; using google_breakpad::test_assembler::kLittleEndian; using google_breakpad::test_assembler::Section; +namespace { + +// Simply calling Section::Append(size, byte) produces a uninteresting pattern +// that tends to get hashed to 0000...0000. This populates the section with +// data to produce better hashes. +void PopulateSection(Section* section, int size, int prime_number) { + for (int i = 0; i < size; i++) + section->Append(1, (i % prime_number) % 256); +} + +} // namespace + TEST(FileIDStripTest, StripSelf) { // Calculate the File ID of this binary using // FileID::ElfFileIdentifier, then make a copy of this binary, @@ -181,3 +193,92 @@ TEST_F(FileIDTest, BuildID) { sizeof(identifier_string)); EXPECT_STREQ(expected_identifier_string, identifier_string); } + +// Test to make sure two files with different text sections produce +// different hashes when not using a build id. +TEST_F(FileIDTest, UniqueHashes32) { + char identifier_string_1[] = + "00000000-0000-0000-0000-000000000000"; + char identifier_string_2[] = + "00000000-0000-0000-0000-000000000000"; + uint8_t identifier_1[sizeof(MDGUID)]; + uint8_t identifier_2[sizeof(MDGUID)]; + + { + ELF elf1(EM_386, ELFCLASS32, kLittleEndian); + Section foo_1(kLittleEndian); + PopulateSection(&foo_1, 32, 5); + elf1.AddSection(".foo", foo_1, SHT_PROGBITS); + Section text_1(kLittleEndian); + PopulateSection(&text_1, 4096, 17); + elf1.AddSection(".text", text_1, SHT_PROGBITS); + elf1.Finish(); + GetElfContents(elf1); + } + + EXPECT_TRUE(FileID::ElfFileIdentifierFromMappedFile(elfdata, identifier_1)); + FileID::ConvertIdentifierToString(identifier_1, identifier_string_1, + sizeof(identifier_string_1)); + + { + ELF elf2(EM_386, ELFCLASS32, kLittleEndian); + Section text_2(kLittleEndian); + Section foo_2(kLittleEndian); + PopulateSection(&foo_2, 32, 5); + elf2.AddSection(".foo", foo_2, SHT_PROGBITS); + PopulateSection(&text_2, 4096, 31); + elf2.AddSection(".text", text_2, SHT_PROGBITS); + elf2.Finish(); + GetElfContents(elf2); + } + + EXPECT_TRUE(FileID::ElfFileIdentifierFromMappedFile(elfdata, identifier_2)); + FileID::ConvertIdentifierToString(identifier_2, identifier_string_2, + sizeof(identifier_string_2)); + + EXPECT_STRNE(identifier_string_1, identifier_string_2); +} + +// Same as UniqueHashes32, for x86-64. +TEST_F(FileIDTest, UniqueHashes64) { + char identifier_string_1[] = + "00000000-0000-0000-0000-000000000000"; + char identifier_string_2[] = + "00000000-0000-0000-0000-000000000000"; + uint8_t identifier_1[sizeof(MDGUID)]; + uint8_t identifier_2[sizeof(MDGUID)]; + + { + ELF elf1(EM_X86_64, ELFCLASS64, kLittleEndian); + Section foo_1(kLittleEndian); + PopulateSection(&foo_1, 32, 5); + elf1.AddSection(".foo", foo_1, SHT_PROGBITS); + Section text_1(kLittleEndian); + PopulateSection(&text_1, 4096, 17); + elf1.AddSection(".text", text_1, SHT_PROGBITS); + elf1.Finish(); + GetElfContents(elf1); + } + + EXPECT_TRUE(FileID::ElfFileIdentifierFromMappedFile(elfdata, identifier_1)); + FileID::ConvertIdentifierToString(identifier_1, identifier_string_1, + sizeof(identifier_string_1)); + + { + ELF elf2(EM_X86_64, ELFCLASS64, kLittleEndian); + Section text_2(kLittleEndian); + Section foo_2(kLittleEndian); + PopulateSection(&foo_2, 32, 5); + elf2.AddSection(".foo", foo_2, SHT_PROGBITS); + PopulateSection(&text_2, 4096, 31); + elf2.AddSection(".text", text_2, SHT_PROGBITS); + elf2.Finish(); + GetElfContents(elf2); + } + + EXPECT_TRUE(FileID::ElfFileIdentifierFromMappedFile(elfdata, identifier_2)); + FileID::ConvertIdentifierToString(identifier_2, identifier_string_2, + sizeof(identifier_string_2)); + + EXPECT_STRNE(identifier_string_1, identifier_string_2); +} -- cgit v1.2.1