Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 25 additions & 11 deletions ddprof-lib/src/main/cpp/symbols_linux.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand All @@ -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;
}

Expand Down Expand Up @@ -1197,11 +1203,19 @@ const uint8_t* SymbolsLinux::findBuildIdInNotes(const void* note_data, size_t no
break;
}

const Elf64_Nhdr* nhdr = reinterpret_cast<const Elf64_Nhdr*>(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<size_t>(3);
size_t desc_size_aligned = (nhdr->n_descsz + 3) & ~static_cast<size_t>(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<size_t>(nhdr.n_namesz) + 3) & ~static_cast<size_t>(3);
size_t desc_size_aligned = (static_cast<size_t>(nhdr.n_descsz) + 3) & ~static_cast<size_t>(3);

// Check bounds using subtraction to avoid overflow
size_t remaining = note_size - offset - sizeof(Elf64_Nhdr);
Expand All @@ -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<const uint8_t*>(data + offset + sizeof(Elf64_Nhdr) + name_size_aligned);
*build_id_len = nhdr->n_descsz;
*build_id_len = nhdr.n_descsz;
return desc;
}
}
Expand Down
54 changes: 54 additions & 0 deletions ddprof-lib/src/test/cpp/elfparser_ut.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@
#include <fcntl.h>
#include <sys/mman.h>
#include <cstring>
#include <cstdlib>
#include <elf.h>
#include <chrono>
#include <thread>

Expand Down Expand Up @@ -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<uint64_t>(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__
Binary file not shown.
Loading