From 278bb8cfff58bfe2b010c0d0af6cb2e28245782c Mon Sep 17 00:00:00 2001 From: Edouard Schweisguth Date: Mon, 1 Jun 2026 11:26:56 +0000 Subject: [PATCH] fix: prevent overflow and misaligned read in build-id parsing --- ddprof-lib/src/main/cpp/symbols_linux.cpp | 36 ++++++++---- ddprof-lib/src/test/cpp/elfparser_ut.cpp | 54 ++++++++++++++++++ .../fuzz_elf/bug3_buildid_note_overflow | Bin 0 -> 120 bytes 3 files changed, 79 insertions(+), 11 deletions(-) create mode 100644 ddprof-lib/src/test/fuzz/corpus/fuzz_elf/bug3_buildid_note_overflow diff --git a/ddprof-lib/src/main/cpp/symbols_linux.cpp b/ddprof-lib/src/main/cpp/symbols_linux.cpp index 3193cff54..3166ef335 100644 --- a/ddprof-lib/src/main/cpp/symbols_linux.cpp +++ b/ddprof-lib/src/main/cpp/symbols_linux.cpp @@ -1144,8 +1144,11 @@ char* SymbolsLinux::extractBuildIdFromMemory(const void* elf_base, size_t elf_si return nullptr; } - // Verify program header table is within file bounds - if (ehdr->e_phoff + ehdr->e_phnum * sizeof(Elf64_Phdr) > elf_size) { + // Verify program header table is within file bounds. Written as subtractions + // so a huge e_phoff cannot wrap the addition and slip past the check, which + // would leave `phdr` pointing outside the mapped image. + if (ehdr->e_phoff > elf_size || + ehdr->e_phnum * sizeof(Elf64_Phdr) > elf_size - ehdr->e_phoff) { return nullptr; } @@ -1160,8 +1163,11 @@ char* SymbolsLinux::extractBuildIdFromMemory(const void* elf_base, size_t elf_si // Search for PT_NOTE segments for (int i = 0; i < ehdr->e_phnum; i++) { if (phdr[i].p_type == PT_NOTE && phdr[i].p_filesz > 0) { - // Ensure note segment is within file bounds - if (phdr[i].p_offset + phdr[i].p_filesz > elf_size) { + // Ensure note segment is within file bounds. Subtraction form avoids + // a u64 overflow in p_offset + p_filesz that would otherwise yield a + // wild note_data pointer passed to findBuildIdInNotes(). + if (phdr[i].p_offset > elf_size || + phdr[i].p_filesz > elf_size - phdr[i].p_offset) { continue; } @@ -1197,11 +1203,19 @@ const uint8_t* SymbolsLinux::findBuildIdInNotes(const void* note_data, size_t no break; } - const Elf64_Nhdr* nhdr = reinterpret_cast(data + offset); + // Copy the note header into an aligned local: note_data is base + + // p_offset and p_offset is attacker-controlled, so dereferencing an + // Elf64_Nhdr* in place could be a misaligned load (UB, and a fault on + // alignment-strict architectures). The size check above guarantees the + // whole header is in bounds. + Elf64_Nhdr nhdr; + memcpy(&nhdr, data + offset, sizeof(nhdr)); - // Calculate aligned sizes (4-byte alignment as per ELF spec) - size_t name_size_aligned = (nhdr->n_namesz + 3) & ~static_cast(3); - size_t desc_size_aligned = (nhdr->n_descsz + 3) & ~static_cast(3); + // Calculate aligned sizes (4-byte alignment as per ELF spec). Promote to + // size_t before the +3 so a near-UINT32_MAX n_namesz/n_descsz cannot wrap + // to a small value and defeat the bounds checks below. + size_t name_size_aligned = (static_cast(nhdr.n_namesz) + 3) & ~static_cast(3); + size_t desc_size_aligned = (static_cast(nhdr.n_descsz) + 3) & ~static_cast(3); // Check bounds using subtraction to avoid overflow size_t remaining = note_size - offset - sizeof(Elf64_Nhdr); @@ -1214,13 +1228,13 @@ const uint8_t* SymbolsLinux::findBuildIdInNotes(const void* note_data, size_t no } // Check if this is a GNU build-id note - if (nhdr->n_type == NT_GNU_BUILD_ID && nhdr->n_namesz > 0 && nhdr->n_descsz > 0) { + if (nhdr.n_type == NT_GNU_BUILD_ID && nhdr.n_namesz > 0 && nhdr.n_descsz > 0) { const char* name = data + offset + sizeof(Elf64_Nhdr); // Verify GNU build-id name (including null terminator) - if (nhdr->n_namesz == 4 && strncmp(name, GNU_BUILD_ID_NAME, 3) == 0 && name[3] == '\0') { + if (nhdr.n_namesz == 4 && strncmp(name, GNU_BUILD_ID_NAME, 3) == 0 && name[3] == '\0') { const uint8_t* desc = reinterpret_cast(data + offset + sizeof(Elf64_Nhdr) + name_size_aligned); - *build_id_len = nhdr->n_descsz; + *build_id_len = nhdr.n_descsz; return desc; } } diff --git a/ddprof-lib/src/test/cpp/elfparser_ut.cpp b/ddprof-lib/src/test/cpp/elfparser_ut.cpp index 3bfc97f75..7fea05090 100644 --- a/ddprof-lib/src/test/cpp/elfparser_ut.cpp +++ b/ddprof-lib/src/test/cpp/elfparser_ut.cpp @@ -16,6 +16,8 @@ #include #include #include +#include +#include #include #include @@ -359,4 +361,56 @@ INSTANTIATE_TEST_SUITE_P( ::testing::Range(3, 21) // This will test delays from 5 to 20 milliseconds inclusive ); #endif + +namespace { + +// Minimal valid ELF64 header; callers set the malformed field afterwards. +Elf64_Ehdr validEhdr() { + Elf64_Ehdr e; + memset(&e, 0, sizeof(e)); + e.e_ident[EI_MAG0] = ELFMAG0; + e.e_ident[EI_MAG1] = ELFMAG1; + e.e_ident[EI_MAG2] = ELFMAG2; + e.e_ident[EI_MAG3] = ELFMAG3; + e.e_ident[EI_CLASS] = ELFCLASS64; + e.e_ident[EI_DATA] = ELFDATA2LSB; + e.e_ident[EI_VERSION] = EV_CURRENT; + e.e_type = ET_DYN; + e.e_machine = EM_X86_64; + e.e_version = EV_CURRENT; + e.e_ehsize = sizeof(Elf64_Ehdr); + e.e_shstrndx = 1; + return e; +} + +} // namespace + +// Regression test for the build-id parser hardening (found by fuzz_elf). +// extractBuildIdFromMemory()'s `p_offset + p_filesz` bounds check could +// overflow, letting findBuildIdInNotes() walk a PT_NOTE past the buffer. The +// buffer is heap-allocated and sized exactly so ASan's redzone catches the +// over-read deterministically. The hardened parser must return cleanly. +TEST(ElfBuildId, noteOffsetOverflow) { + Elf64_Ehdr e = validEhdr(); + e.e_phoff = sizeof(Elf64_Ehdr); + e.e_phentsize = sizeof(Elf64_Phdr); + e.e_phnum = 1; + + Elf64_Phdr p; + memset(&p, 0, sizeof(p)); + p.p_type = PT_NOTE; + p.p_offset = 0x70; // inside the buffer + p.p_filesz = static_cast(8) - p.p_offset; // sum wraps to 8 (< size) + + const size_t size = sizeof(e) + sizeof(p); // 120 bytes + char* buf = new char[size]; // exact size -> redzone right after + memcpy(buf, &e, sizeof(e)); + memcpy(buf + sizeof(e), &p, sizeof(p)); + + size_t build_id_len = 0; + char* id = SymbolsLinux::extractBuildIdFromMemory(buf, size, &build_id_len); + free(id); // hardened parser returns nullptr; the point is that it must not crash + delete[] buf; +} + #endif //__linux__ diff --git a/ddprof-lib/src/test/fuzz/corpus/fuzz_elf/bug3_buildid_note_overflow b/ddprof-lib/src/test/fuzz/corpus/fuzz_elf/bug3_buildid_note_overflow new file mode 100644 index 0000000000000000000000000000000000000000..f079ff52f37f9673e61fb2a3d962df0718f4ccca GIT binary patch literal 120 zcmb<-^>JfjWMqH=W(GS31doBi0VaV&J1|&4)i5%!K*bA?Er3pu-AW literal 0 HcmV?d00001