diff --git a/news/360.bugfix.rst b/news/360.bugfix.rst new file mode 100644 index 00000000..8f1ad4f5 --- /dev/null +++ b/news/360.bugfix.rst @@ -0,0 +1 @@ +Explicitly reject core files whose endianness or word size does not match PyStack's. These would never have worked reliably, but until now the reason wasn't clearly surfaced to the user. diff --git a/src/pystack/_pystack/corefile.cpp b/src/pystack/_pystack/corefile.cpp index d55dbf95..7fb27fb3 100644 --- a/src/pystack/_pystack/corefile.cpp +++ b/src/pystack/_pystack/corefile.cpp @@ -245,7 +245,7 @@ parseCoreSiginfo(const NoteData& note_data, CoreCrashInfo* result) } const size_t int_size = gelf_fsize(note_data.elf, ELF_T_WORD, 1, EV_CURRENT); - assert(int_size > 0); + const size_t addr_size = gelf_fsize(note_data.elf, ELF_T_ADDR, 1, EV_CURRENT); const char* ptr = static_cast(note_data.data->d_buf); read_obj(&ptr, &result->si_signo, int_size); @@ -263,8 +263,6 @@ parseCoreSiginfo(const NoteData& note_data, CoreCrashInfo* result) case SIGFPE: case SIGSEGV: case SIGBUS: { - const size_t addr_size = gelf_fsize(note_data.elf, ELF_T_ADDR, 1, EV_CURRENT); - assert(addr_size > 0); read_obj(&ptr, &result->failed_addr, addr_size); break; } @@ -293,13 +291,12 @@ parseCoreFileNote(Elf* core, const NoteData& note_data, std::vector(data->d_buf); const char* end = static_cast(data->d_buf) + data->d_size; - uint64_t count, page_size; + uintptr_t count, page_size; read_obj(&ptr, &count, ulong_size); read_obj(&ptr, &page_size, ulong_size); - size_t addrsize = gelf_fsize(core, ELF_T_ADDR, 1, EV_CURRENT); - size_t entry_size = 3 * addrsize; // mstart, mend, moffset - uint64_t maxcount = (size_t)(end - ptr) / entry_size; + const size_t entry_size = 3 * ulong_size; // mstart, mend, moffset + const size_t maxcount = static_cast(end - ptr) / entry_size; if (count > maxcount) { LOG(ERROR) << "Failed to parse file note data: invalid number of entries"; return StatusCode::ERROR; @@ -311,7 +308,7 @@ parseCoreFileNote(Elf* core, const NoteData& note_data, std::vector 0); + if (auxv_size == 0) { + LOG(ERROR) << "Cannot determine the size of an auxv entry for ELF file"; + return StatusCode::ERROR; + } const size_t nauxv = note_data.descriptor_size / auxv_size; for (size_t i = 0; i < nauxv; ++i) { GElf_auxv_t av_mem; diff --git a/src/pystack/_pystack/elf_common.cpp b/src/pystack/_pystack/elf_common.cpp index 96e43f25..012d1707 100644 --- a/src/pystack/_pystack/elf_common.cpp +++ b/src/pystack/_pystack/elf_common.cpp @@ -1,3 +1,4 @@ +#include #include #include #include @@ -84,6 +85,25 @@ CoreFileAnalyzer::CoreFileAnalyzer( throw ElfAnalyzerError("Cannot read elf file"); } + size_t ident_size = 0; + const char* ident = elf_getident(d_elf.get(), &ident_size); + if (ident == nullptr || ident_size <= EI_DATA) { + close(d_fd); + throw ElfAnalyzerError("Cannot read the ELF header of '" + d_filename + "'"); + } + + const auto core_endianness = static_cast(ident[EI_DATA]); + const bool endianness_matches = + (core_endianness == ELFDATA2LSB && std::endian::native == std::endian::little) + || (core_endianness == ELFDATA2MSB && std::endian::native == std::endian::big); + + const auto core_pointer_size = gelf_fsize(d_elf.get(), ELF_T_ADDR, 1, EV_CURRENT); + const bool pointer_size_matches = sizeof(uintptr_t) == core_pointer_size; + if (!endianness_matches || !pointer_size_matches) { + close(d_fd); + throw ElfAnalyzerError("The core file '" + d_filename + "' has an unsupported format."); + } + std::memset(&d_callbacks, 0, sizeof(d_callbacks)); d_callbacks.find_elf = pystack_find_elf; d_callbacks.find_debuginfo = dwfl_standard_find_debuginfo; diff --git a/tests/integration/test_core_analyzer.py b/tests/integration/test_core_analyzer.py index bcaae74c..1fb06d39 100644 --- a/tests/integration/test_core_analyzer.py +++ b/tests/integration/test_core_analyzer.py @@ -561,6 +561,25 @@ def test_core_analizer_raises_when_an_invalid_core_is_provided(tmpdir: Path) -> list(get_process_threads_for_core(Path(not_a_core), Path(sys.executable))) +@pytest.mark.parametrize( + ("ident_index", "incompatible_value"), + [ + (5, 2 if sys.byteorder == "little" else 1), # EI_DATA + (4, 1 if sys.maxsize > 2**32 else 2), # EI_CLASS + ], +) +def test_core_analyzer_rejects_incompatible_core_format( + tmpdir: Path, ident_index: int, incompatible_value: int +) -> None: + core = bytearray((CORE_FILE_PATHS / "segfault.core").read_bytes()) + core[ident_index] = incompatible_value + incompatible_core = Path(tmpdir) / "incompatible.core" + incompatible_core.write_bytes(core) + + with pytest.raises(RuntimeError, match="unsupported format"): + CoreFileAnalyzer(str(incompatible_core)) + + def test_invalid_method_for_get_process_threads_for_core(): # GIVEN devnull = Path("/dev/null")