From a6a81e39e7a71ef32514dd9c1f8d8cd005e3a4df Mon Sep 17 00:00:00 2001 From: Nick Biryulin Date: Sat, 29 Mar 2025 23:26:31 +0300 Subject: [PATCH] Fix use-after-free when non-lazily processing corrupted sections Right now, return value of sec->load() is not checked on initial ELF load. However, if the stream is corrupt, it's load_data() call can fail, meaning that is_loaded flag will be left false. Despite that, problematic section will be left in the file. When reading sections after all (like in elfdump example), user might call get_data() on the section, which will in turn try to load_data() again. However, if the file was loaded non-lazily, the stream set in sec->load() will be freed by this point, thus causing a problem. Initially I wanted to just discard corrupted sections, however, since there are test cases for corrupted elf processing, I opted to add a can_be_loaded field to section object. If load_data failed, it is set to false, thus preventing any broken reads from occuring. --- elfio/elfio.hpp | 3 +++ elfio/elfio_section.hpp | 16 +++++++++++++--- 2 files changed, 16 insertions(+), 3 deletions(-) diff --git a/elfio/elfio.hpp b/elfio/elfio.hpp index 119951b0c..24a911678 100644 --- a/elfio/elfio.hpp +++ b/elfio/elfio.hpp @@ -560,6 +560,9 @@ class elfio for ( Elf_Half i = 0; i < num; ++i ) { section* sec = create_section(); + + // Load return value is ignored here + // This allows retrieval of information from corrupted sections sec->load( stream, static_cast( offset ) + static_cast( i ) * entry_size, diff --git a/elfio/elfio_section.hpp b/elfio/elfio_section.hpp index 30e083d15..cfd440817 100644 --- a/elfio/elfio_section.hpp +++ b/elfio/elfio_section.hpp @@ -228,8 +228,16 @@ template class section_impl : public section */ const char* get_data() const override { - if ( !is_loaded ) { - load_data(); + // If data load failed, the stream is corrupt + // When lazy loading, attempts to call get_data() on it after initial load are useless + // When loading non-lazily, that load_data() will attempt to read data from + // the stream specified on load() call, which might be freed by this point + if ( !is_loaded && can_be_loaded ) { + bool res = load_data(); + + if ( !res ) { + can_be_loaded = false; + } } return data.get(); } @@ -434,7 +442,7 @@ template class section_impl : public section stream.read( reinterpret_cast( &header ), sizeof( header ) ); if ( !( is_lazy || is_loaded ) ) { - bool ret = load_data(); + bool ret = get_data(); if ( is_compressed() ) { Elf_Xword size = get_size(); @@ -588,6 +596,8 @@ template class section_impl : public section false; /**< Flag indicating if lazy loading is enabled. */ mutable bool is_loaded = false; /**< Flag indicating if the data is loaded. */ + mutable bool can_be_loaded = + true; /**< Flag indicating if the data can loaded. This is not the case if the section is corrupted. */ }; } // namespace ELFIO