From de382da106e2d1fb38ad4cc23a1496230f739edf Mon Sep 17 00:00:00 2001 From: sstamenk Date: Mon, 31 Aug 2026 13:25:51 +0200 Subject: [PATCH 1/6] Validate note accessor bounds --- elfio/elfio_note.hpp | 97 +++++++++++++++++++----------- tests/CMakeLists.txt | 1 + tests/ELFIONoteBoundsTest.cpp | 107 ++++++++++++++++++++++++++++++++++ 3 files changed, 172 insertions(+), 33 deletions(-) create mode 100644 tests/ELFIONoteBoundsTest.cpp diff --git a/elfio/elfio_note.hpp b/elfio/elfio_note.hpp index c246fb527..c0d8d1fde 100644 --- a/elfio/elfio_note.hpp +++ b/elfio/elfio_note.hpp @@ -23,6 +23,8 @@ THE SOFTWARE. #ifndef ELFIO_NOTE_HPP #define ELFIO_NOTE_HPP +#include + namespace ELFIO { //------------------------------------------------------------------------------ @@ -76,34 +78,56 @@ class note_section_accessor_template char*& desc, Elf_Word& descSize ) const { - if ( index >= ( notes->*F_get_size )() ) { + if ( index >= note_start_positions.size() ) { return false; } - const char* pData = notes->get_data() + note_start_positions[index]; - int align = sizeof( Elf_Word ); + const char* data = notes->get_data(); + Elf_Xword data_size = ( notes->*F_get_size )(); + Elf_Xword position = note_start_positions[index]; + Elf_Xword align = sizeof( Elf_Word ); + Elf_Xword header_size = 3 * align; + if ( data == nullptr || position > data_size || + header_size > data_size - position ) { + return false; + } const auto& convertor = elf_file.get_convertor(); - type = - ( *convertor )( *(const Elf_Word*)( pData + 2 * (size_t)align ) ); - Elf_Word namesz = ( *convertor )( *(const Elf_Word*)( pData ) ); - descSize = - ( *convertor )( *(const Elf_Word*)( pData + sizeof( namesz ) ) ); - - Elf_Xword max_name_size = - ( notes->*F_get_size )() - note_start_positions[index]; - if ( namesz < 1 || namesz > max_name_size || - (Elf_Xword)namesz + descSize > max_name_size ) { + const char* pData = data + position; + Elf_Word raw_namesz; + Elf_Word raw_descsz; + Elf_Word raw_type; + std::memcpy( &raw_namesz, pData, sizeof( raw_namesz ) ); + std::memcpy( &raw_descsz, pData + align, sizeof( raw_descsz ) ); + std::memcpy( &raw_type, pData + 2 * align, sizeof( raw_type ) ); + + Elf_Word namesz = ( *convertor )( raw_namesz ); + Elf_Word parsed_descsz = ( *convertor )( raw_descsz ); + Elf_Word parsed_type = ( *convertor )( raw_type ); + Elf_Xword padded_name = + ( static_cast( namesz ) + align - 1 ) / align * align; + Elf_Xword padded_desc = + ( static_cast( parsed_descsz ) + align - 1 ) / align * + align; + Elf_Xword remaining = data_size - position - header_size; + if ( padded_name > remaining || + padded_desc > remaining - padded_name ) { return false; } - name.assign( pData + 3 * (size_t)align, namesz - 1 ); - if ( 0 == descSize ) { + + type = parsed_type; + descSize = parsed_descsz; + if ( namesz == 0 ) { + name.clear(); + } + else { + name.assign( pData + header_size, namesz - 1 ); + } + if ( 0 == parsed_descsz ) { desc = nullptr; } else { - desc = const_cast( pData + 3 * (size_t)align + - ( ( namesz + align - 1 ) / align ) * - (size_t)align ); + desc = const_cast( pData + header_size + padded_name ); } return true; @@ -165,24 +189,31 @@ class note_section_accessor_template return; } - Elf_Word align = sizeof( Elf_Word ); - while ( current + (Elf_Xword)3 * align <= size ) { - Elf_Word namesz = - ( *convertor )( *(const Elf_Word*)( data + current ) ); - Elf_Word descsz = ( *convertor )( - *(const Elf_Word*)( data + current + sizeof( namesz ) ) ); - Elf_Word advance = - (Elf_Xword)3 * sizeof( Elf_Word ) + - ( ( namesz + align - 1 ) / align ) * (Elf_Xword)align + - ( ( descsz + align - 1 ) / align ) * (Elf_Xword)align; - if ( namesz < size && descsz < size && current + advance <= size ) { - note_start_positions.emplace_back( current ); - } - else { + Elf_Xword align = sizeof( Elf_Word ); + Elf_Xword header_size = 3 * align; + while ( current <= size && header_size <= size - current ) { + Elf_Word raw_namesz; + Elf_Word raw_descsz; + std::memcpy( &raw_namesz, data + current, sizeof( raw_namesz ) ); + std::memcpy( &raw_descsz, data + current + sizeof( raw_namesz ), + sizeof( raw_descsz ) ); + + Elf_Word namesz = ( *convertor )( raw_namesz ); + Elf_Word descsz = ( *convertor )( raw_descsz ); + Elf_Xword padded_name = + ( static_cast( namesz ) + align - 1 ) / align * + align; + Elf_Xword padded_desc = + ( static_cast( descsz ) + align - 1 ) / align * + align; + Elf_Xword remaining = size - current - header_size; + if ( padded_name > remaining || + padded_desc > remaining - padded_name ) { break; } - current += advance; + note_start_positions.emplace_back( current ); + current += header_size + padded_name + padded_desc; } } diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index 1c5928531..0001b924b 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -65,6 +65,7 @@ add_executable( ELFIOTest.cpp ELFIOTest1.cpp ELFIOTest2.cpp + ELFIONoteBoundsTest.cpp ARIOTest.cpp) target_link_libraries( diff --git a/tests/ELFIONoteBoundsTest.cpp b/tests/ELFIONoteBoundsTest.cpp new file mode 100644 index 000000000..79f7c1299 --- /dev/null +++ b/tests/ELFIONoteBoundsTest.cpp @@ -0,0 +1,107 @@ +/* +Copyright (C) 2001-present by Serge Lamikhov-Center + +Permission is hereby granted, free of charge, to any person obtaining a copy +of this software and associated documentation files (the "Software"), to deal +in the Software without restriction, including without limitation the rights +to use, copy, modify, merge, publish, distribute, sublicense, and/or sell +copies of the Software, and to permit persons to whom the Software is +furnished to do so, subject to the following conditions: + +The above copyright notice and this permission notice shall be included in +all copies or substantial portions of the Software. + +THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE +AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, +OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN +THE SOFTWARE. +*/ + +#include +#include + +#include +#include + +using namespace ELFIO; + +TEST( ELFIONoteBoundsTest, RejectsOutOfRangeIndex ) +{ + elfio file; + file.create( ELFCLASS64, ELFDATA2LSB ); + section* notes = file.sections.add( ".note.test" ); + notes->set_type( SHT_NOTE ); + + note_section_accessor accessor( file, notes ); + std::uint32_t payload = 0x12345678; + accessor.add_note( 1, "test", reinterpret_cast( &payload ), + sizeof( payload ) ); + ASSERT_EQ( accessor.get_notes_num(), 1U ); + + Elf_Word type = 0; + std::string name = "unchanged"; + char* desc = nullptr; + Elf_Word descSize = 0; + EXPECT_FALSE( accessor.get_note( 1, type, name, desc, descSize ) ); + EXPECT_EQ( name, "unchanged" ); +} + +TEST( ELFIONoteBoundsTest, RejectsTruncatedPayload ) +{ + elfio file; + file.create( ELFCLASS64, ELFDATA2LSB ); + section* notes = file.sections.add( ".note.test" ); + notes->set_type( SHT_NOTE ); + + const std::uint32_t header[] = { 12, 0, 1 }; + notes->set_data( reinterpret_cast( header ), + sizeof( header ) ); + + note_section_accessor accessor( file, notes ); + EXPECT_EQ( accessor.get_notes_num(), 0U ); +} + +TEST( ELFIONoteBoundsTest, AcceptsNamelessNoteAndContinuesParsing ) +{ + elfio file; + file.create( ELFCLASS64, ELFDATA2LSB ); + section* notes = file.sections.add( ".note.test" ); + notes->set_type( SHT_NOTE ); + + std::string data; + const auto& convertor = file.get_convertor(); + auto append_word = [&]( Elf_Word value ) { + value = ( *convertor )( value ); + data.append( reinterpret_cast( &value ), sizeof( value ) ); + }; + + append_word( 0 ); + append_word( 0 ); + append_word( 1 ); + append_word( 4 ); + append_word( 0 ); + append_word( 2 ); + data.append( "GNU\0", 4 ); + notes->set_data( data ); + + note_section_accessor accessor( file, notes ); + ASSERT_EQ( accessor.get_notes_num(), 2U ); + + Elf_Word type = 0; + std::string name = "not empty"; + char marker = 0; + char* desc = ▮ + Elf_Word descSize = 1; + ASSERT_TRUE( accessor.get_note( 0, type, name, desc, descSize ) ); + EXPECT_EQ( type, 1U ); + EXPECT_TRUE( name.empty() ); + EXPECT_EQ( desc, nullptr ); + EXPECT_EQ( descSize, 0U ); + + ASSERT_TRUE( accessor.get_note( 1, type, name, desc, descSize ) ); + EXPECT_EQ( type, 2U ); + EXPECT_EQ( name, "GNU" ); +} From 23839d1dfa8d18d26f3096289e7897ba75f85182 Mon Sep 17 00:00:00 2001 From: sstamenk Date: Thu, 17 Sep 2026 19:13:46 +0200 Subject: [PATCH 2/6] Share note decoding and extend accessor tests Centralize record validation and check owner string termination. Cover both ELF classes, byte orders, section and segment access, and data mutation. Keep test registration compatible with the hash lookup changes. --- elfio/elfio_note.hpp | 150 +++++++++---------- tests/CMakeLists.txt | 2 +- tests/ELFIONoteBoundsTest.cpp | 269 ++++++++++++++++++++++++++-------- 3 files changed, 278 insertions(+), 143 deletions(-) diff --git a/elfio/elfio_note.hpp b/elfio/elfio_note.hpp index c0d8d1fde..29fdb82df 100644 --- a/elfio/elfio_note.hpp +++ b/elfio/elfio_note.hpp @@ -82,54 +82,19 @@ class note_section_accessor_template return false; } - const char* data = notes->get_data(); - Elf_Xword data_size = ( notes->*F_get_size )(); - Elf_Xword position = note_start_positions[index]; - Elf_Xword align = sizeof( Elf_Word ); - Elf_Xword header_size = 3 * align; - if ( data == nullptr || position > data_size || - header_size > data_size - position ) { - return false; - } - - const auto& convertor = elf_file.get_convertor(); - const char* pData = data + position; - Elf_Word raw_namesz; - Elf_Word raw_descsz; - Elf_Word raw_type; - std::memcpy( &raw_namesz, pData, sizeof( raw_namesz ) ); - std::memcpy( &raw_descsz, pData + align, sizeof( raw_descsz ) ); - std::memcpy( &raw_type, pData + 2 * align, sizeof( raw_type ) ); - - Elf_Word namesz = ( *convertor )( raw_namesz ); - Elf_Word parsed_descsz = ( *convertor )( raw_descsz ); - Elf_Word parsed_type = ( *convertor )( raw_type ); - Elf_Xword padded_name = - ( static_cast( namesz ) + align - 1 ) / align * align; - Elf_Xword padded_desc = - ( static_cast( parsed_descsz ) + align - 1 ) / align * - align; - Elf_Xword remaining = data_size - position - header_size; - if ( padded_name > remaining || - padded_desc > remaining - padded_name ) { + const char* data = notes->get_data(); + const Elf_Xword data_size = ( notes->*F_get_size )(); + note_record record; + // The section may have changed since its note offsets were collected. + if ( !read_note( data, data_size, note_start_positions[index], + record ) ) { return false; } - type = parsed_type; - descSize = parsed_descsz; - if ( namesz == 0 ) { - name.clear(); - } - else { - name.assign( pData + header_size, namesz - 1 ); - } - if ( 0 == parsed_descsz ) { - desc = nullptr; - } - else { - desc = const_cast( pData + header_size + padded_name ); - } - + name.assign( record.name, record.name_size ); + type = record.type; + descSize = record.descriptor_size; + desc = const_cast( record.descriptor ); return true; } @@ -173,47 +138,76 @@ class note_section_accessor_template } private: + struct note_record + { + Elf_Word type; + const char* name; + Elf_Word name_size; + const char* descriptor; + Elf_Word descriptor_size; + Elf_Xword total_size; + }; + + static Elf_Xword padded_size( Elf_Word size ) + { + constexpr Elf_Xword alignment = sizeof( Elf_Word ); + return ( static_cast( size ) + alignment - 1 ) / alignment * + alignment; + } + + // Decode one complete record. Scanning and lookup share these checks. + bool read_note( const char* data, + Elf_Xword data_size, + Elf_Xword position, + note_record& record ) const + { + constexpr Elf_Xword header_size = 3 * sizeof( Elf_Word ); + if ( data == nullptr || position > data_size || + header_size > data_size - position ) { + return false; + } + + Elf_Word header[3]; + std::memcpy( header, data + position, sizeof( header ) ); + const auto& convertor = elf_file.get_convertor(); + const Elf_Word namesz = ( *convertor )( header[0] ); + const Elf_Word descsz = ( *convertor )( header[1] ); + const Elf_Xword padded_name = padded_size( namesz ); + const Elf_Xword padded_desc = padded_size( descsz ); + const Elf_Xword remaining = data_size - position - header_size; + if ( padded_name > remaining || + padded_desc > remaining - padded_name ) { + return false; + } + + const char* name = data + position + header_size; + if ( namesz != 0 && name[namesz - 1] != '\0' ) { + return false; + } + record.type = ( *convertor )( header[2] ); + record.name = name; + record.name_size = namesz == 0 ? 0 : namesz - 1; + record.descriptor = descsz == 0 ? nullptr : name + padded_name; + record.descriptor_size = descsz; + record.total_size = header_size + padded_name + padded_desc; + return true; + } + //------------------------------------------------------------------------------ //! \brief Process the section to extract note start positions void process_section() { - const auto& convertor = elf_file.get_convertor(); - const char* data = notes->get_data(); - Elf_Xword size = ( notes->*F_get_size )(); - Elf_Xword current = 0; - note_start_positions.clear(); - - // Is it empty? - if ( nullptr == data || 0 == size ) { + if ( notes == nullptr ) { return; } - - Elf_Xword align = sizeof( Elf_Word ); - Elf_Xword header_size = 3 * align; - while ( current <= size && header_size <= size - current ) { - Elf_Word raw_namesz; - Elf_Word raw_descsz; - std::memcpy( &raw_namesz, data + current, sizeof( raw_namesz ) ); - std::memcpy( &raw_descsz, data + current + sizeof( raw_namesz ), - sizeof( raw_descsz ) ); - - Elf_Word namesz = ( *convertor )( raw_namesz ); - Elf_Word descsz = ( *convertor )( raw_descsz ); - Elf_Xword padded_name = - ( static_cast( namesz ) + align - 1 ) / align * - align; - Elf_Xword padded_desc = - ( static_cast( descsz ) + align - 1 ) / align * - align; - Elf_Xword remaining = size - current - header_size; - if ( padded_name > remaining || - padded_desc > remaining - padded_name ) { - break; - } - + const char* data = notes->get_data(); + const Elf_Xword data_size = ( notes->*F_get_size )(); + Elf_Xword current = 0; + note_record record; + while ( read_note( data, data_size, current, record ) ) { note_start_positions.emplace_back( current ); - current += header_size + padded_name + padded_desc; + current += record.total_size; } } diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index 0001b924b..841bdf62d 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -62,10 +62,10 @@ configure_file(runELFtests ${CMAKE_CURRENT_BINARY_DIR}/runELFtests COPYONLY) add_executable( ELFIOTest + ELFIONoteBoundsTest.cpp ELFIOTest.cpp ELFIOTest1.cpp ELFIOTest2.cpp - ELFIONoteBoundsTest.cpp ARIOTest.cpp) target_link_libraries( diff --git a/tests/ELFIONoteBoundsTest.cpp b/tests/ELFIONoteBoundsTest.cpp index 79f7c1299..201a726e3 100644 --- a/tests/ELFIONoteBoundsTest.cpp +++ b/tests/ELFIONoteBoundsTest.cpp @@ -20,88 +20,229 @@ OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. */ -#include +#include +#include #include +#include #include #include using namespace ELFIO; -TEST( ELFIONoteBoundsTest, RejectsOutOfRangeIndex ) +namespace { + +void append_word( std::string& data, const elfio& file, Elf_Word value ) +{ + value = ( *file.get_convertor() )( value ); + data.append( reinterpret_cast( &value ), sizeof( value ) ); +} + +std::string make_note( const elfio& file, + Elf_Word type, + const std::string& name, + const std::string& descriptor = {} ) +{ + // Names include their declared terminator; an empty name encodes namesz=0. + std::string data; + append_word( data, file, static_cast( name.size() ) ); + append_word( data, file, static_cast( descriptor.size() ) ); + append_word( data, file, type ); + data.append( name ); + data.append( ( 4 - name.size() % 4 ) % 4, '\0' ); + data.append( descriptor ); + data.append( ( 4 - descriptor.size() % 4 ) % 4, '\0' ); + return data; +} + +template +void expect_rejected( const Accessor& reader, Elf_Word index ) { - elfio file; - file.create( ELFCLASS64, ELFDATA2LSB ); - section* notes = file.sections.add( ".note.test" ); - notes->set_type( SHT_NOTE ); - - note_section_accessor accessor( file, notes ); - std::uint32_t payload = 0x12345678; - accessor.add_note( 1, "test", reinterpret_cast( &payload ), - sizeof( payload ) ); - ASSERT_EQ( accessor.get_notes_num(), 1U ); - - Elf_Word type = 0; - std::string name = "unchanged"; - char* desc = nullptr; - Elf_Word descSize = 0; - EXPECT_FALSE( accessor.get_note( 1, type, name, desc, descSize ) ); + Elf_Word type = 11; + std::string name = "unchanged"; + char marker = 0; + char* descriptor = ▮ + Elf_Word descriptor_size = 13; + EXPECT_FALSE( + reader.get_note( index, type, name, descriptor, descriptor_size ) ); + EXPECT_EQ( type, 11U ); EXPECT_EQ( name, "unchanged" ); + EXPECT_EQ( descriptor, &marker ); + EXPECT_EQ( descriptor_size, 13U ); } -TEST( ELFIONoteBoundsTest, RejectsTruncatedPayload ) +template +void expect_note( const Accessor& reader, + Elf_Word index, + Elf_Word expected_type, + const std::string& expected_name, + const std::string& expected_descriptor ) { - elfio file; - file.create( ELFCLASS64, ELFDATA2LSB ); - section* notes = file.sections.add( ".note.test" ); - notes->set_type( SHT_NOTE ); + Elf_Word type = 0; + std::string name; + char* descriptor = nullptr; + Elf_Word descriptor_size = 0; + ASSERT_TRUE( + reader.get_note( index, type, name, descriptor, descriptor_size ) ); + EXPECT_EQ( type, expected_type ); + EXPECT_EQ( name, expected_name ); + ASSERT_EQ( descriptor_size, expected_descriptor.size() ); + if ( expected_descriptor.empty() ) { + EXPECT_EQ( descriptor, nullptr ); + } + else { + ASSERT_NE( descriptor, nullptr ); + EXPECT_EQ( std::string( descriptor, descriptor_size ), + expected_descriptor ); + } +} - const std::uint32_t header[] = { 12, 0, 1 }; - notes->set_data( reinterpret_cast( header ), - sizeof( header ) ); +class ELFIONoteBoundsTest + : public ::testing::TestWithParam> +{ + protected: + void SetUp() override + { + file.create( std::get<0>( GetParam() ), std::get<1>( GetParam() ) ); + notes = file.sections.add( ".note.test" ); + notes->set_type( SHT_NOTE ); + notes->set_addr_align( 4 ); + } + + elfio file; + section* notes = nullptr; +}; + +TEST_P( ELFIONoteBoundsTest, RejectsOutOfRangeIndex ) +{ + note_section_accessor reader( file, notes ); + expect_rejected( reader, 0 ); + reader.add_note( 7, "test", "abc", 3 ); + ASSERT_EQ( reader.get_notes_num(), 1U ); + expect_note( reader, 0, 7, "test", "abc" ); + expect_rejected( reader, 1 ); + expect_rejected( reader, std::numeric_limits::max() ); +} - note_section_accessor accessor( file, notes ); - EXPECT_EQ( accessor.get_notes_num(), 0U ); +TEST_P( ELFIONoteBoundsTest, RejectsTruncatedPayloadAndPadding ) +{ + const auto data = make_note( file, 7, std::string( "test\0", 5 ), "abc" ); + for ( size_t length = 0; length < data.size(); ++length ) { + SCOPED_TRACE( length ); + notes->set_data( data.data(), length ); + note_section_accessor reader( file, notes ); + EXPECT_EQ( reader.get_notes_num(), 0U ); + expect_rejected( reader, 0 ); + } } -TEST( ELFIONoteBoundsTest, AcceptsNamelessNoteAndContinuesParsing ) +TEST_P( ELFIONoteBoundsTest, RejectsOversizedLengths ) { - elfio file; - file.create( ELFCLASS64, ELFDATA2LSB ); - section* notes = file.sections.add( ".note.test" ); - notes->set_type( SHT_NOTE ); + for ( bool oversized_name : { false, true } ) { + std::string data; + const auto maximum = std::numeric_limits::max(); + append_word( data, file, oversized_name ? maximum : 4 ); + append_word( data, file, oversized_name ? 0 : maximum ); + append_word( data, file, 7 ); + data.append( "GNU\0", 4 ); + notes->set_data( data ); + note_section_accessor reader( file, notes ); + EXPECT_EQ( reader.get_notes_num(), 0U ); + expect_rejected( reader, 0 ); + } +} - std::string data; - const auto& convertor = file.get_convertor(); - auto append_word = [&]( Elf_Word value ) { - value = ( *convertor )( value ); - data.append( reinterpret_cast( &value ), sizeof( value ) ); - }; - - append_word( 0 ); - append_word( 0 ); - append_word( 1 ); - append_word( 4 ); - append_word( 0 ); - append_word( 2 ); - data.append( "GNU\0", 4 ); +TEST_P( ELFIONoteBoundsTest, AcceptsNamelessNoteAndContinuesParsing ) +{ + for ( const std::string& payload : + { std::string{}, std::string{ "abc" } } ) { + const auto data = make_note( file, 1, {}, payload ) + + make_note( file, 2, std::string( "GNU\0", 4 ), "xy" ); + notes->set_data( data ); + note_section_accessor reader( file, notes ); + ASSERT_EQ( reader.get_notes_num(), 2U ); + expect_note( reader, 0, 1, "", payload ); + expect_note( reader, 1, 2, "GNU", "xy" ); + } +} + +TEST_P( ELFIONoteBoundsTest, RevalidatesCachedOffsetsWithoutChangingOutputs ) +{ + const auto first = make_note( file, 1, std::string( "GNU\0", 4 ) ); + const auto data = first + make_note( file, 2, {}, "abc" ); + notes->set_data( data ); + note_section_accessor reader( file, notes ); + ASSERT_EQ( reader.get_notes_num(), 2U ); + for ( size_t length : { size_t( 0 ), size_t( 1 ), first.size() + 5 } ) { + notes->set_data( data.data(), length ); + expect_rejected( reader, 1 ); + } + std::string oversized = first; + append_word( oversized, file, 0 ); + append_word( oversized, file, std::numeric_limits::max() ); + append_word( oversized, file, 2 ); + notes->set_data( oversized ); + expect_rejected( reader, 1 ); + notes->set_data( data ); + expect_note( reader, 1, 2, "", "abc" ); +} + +TEST_P( ELFIONoteBoundsTest, RejectsUnterminatedOwnerName ) +{ + auto data = make_note( file, 7, std::string( "GNU\0", 4 ) ); notes->set_data( data ); + note_section_accessor cached( file, notes ); + data.back() = 'x'; + notes->set_data( data ); + expect_rejected( cached, 0 ); + note_section_accessor fresh( file, notes ); + EXPECT_EQ( fresh.get_notes_num(), 0U ); +} + +TEST_P( ELFIONoteBoundsTest, ReadsSectionAndSegmentNotes ) +{ + notes->set_data( make_note( file, 7, std::string( "GNU\0", 4 ), "abc" ) ); + segment* note_segment = file.segments.add(); + note_segment->set_type( PT_NOTE ); + note_segment->set_align( 4 ); + note_segment->add_section_index( notes->get_index(), 4 ); + std::stringstream stream( std::ios::in | std::ios::out | std::ios::binary ); + ASSERT_TRUE( file.save( stream ) ); + stream.seekg( 0 ); + elfio loaded; + ASSERT_TRUE( loaded.load( stream ) ); + const_note_section_accessor section_reader( loaded, + loaded.sections[".note.test"] ); + // A note segment's file size, not its memory size, bounds the payload. + loaded.segments[0]->set_memory_size( loaded.segments[0]->get_file_size() + + 512 ); + const_note_segment_accessor segment_reader( loaded, loaded.segments[0] ); + ASSERT_EQ( section_reader.get_notes_num(), 1U ); + ASSERT_EQ( segment_reader.get_notes_num(), 1U ); + expect_note( section_reader, 0, 7, "GNU", "abc" ); + expect_note( segment_reader, 0, 7, "GNU", "abc" ); + expect_rejected( segment_reader, 1 ); +} - note_section_accessor accessor( file, notes ); - ASSERT_EQ( accessor.get_notes_num(), 2U ); - - Elf_Word type = 0; - std::string name = "not empty"; - char marker = 0; - char* desc = ▮ - Elf_Word descSize = 1; - ASSERT_TRUE( accessor.get_note( 0, type, name, desc, descSize ) ); - EXPECT_EQ( type, 1U ); - EXPECT_TRUE( name.empty() ); - EXPECT_EQ( desc, nullptr ); - EXPECT_EQ( descSize, 0U ); - - ASSERT_TRUE( accessor.get_note( 1, type, name, desc, descSize ) ); - EXPECT_EQ( type, 2U ); - EXPECT_EQ( name, "GNU" ); +TEST_P( ELFIONoteBoundsTest, ReadsUnalignedHeaders ) +{ + struct note_view + { + std::string storage; + const char* get_data() const { return storage.data() + 1; } + Elf_Xword get_size() const { return storage.size() - 1; } + } view; + view.storage = "x" + make_note( file, 7, std::string( "GNU\0", 4 ), "abc" ); + note_section_accessor_template reader( + file, &view ); + ASSERT_EQ( reader.get_notes_num(), 1U ); + expect_note( reader, 0, 7, "GNU", "abc" ); } + +INSTANTIATE_TEST_SUITE_P( + Formats, + ELFIONoteBoundsTest, + ::testing::Combine( ::testing::Values( ELFCLASS32, ELFCLASS64 ), + ::testing::Values( ELFDATA2LSB, ELFDATA2MSB ) ) ); + +} // namespace From 57f1676ce5e0a8fb3a37d77a3688e09cccb0c6e0 Mon Sep 17 00:00:00 2001 From: sstamenk Date: Tue, 13 Oct 2026 13:52:53 +0200 Subject: [PATCH 3/6] Do not claim a descriptor for notes added without data add_note() wrote the caller's descriptor size into the record header even when no descriptor data was supplied and none was appended. The resulting record failed validation, and scanning stopped there, hiding every note added after it. Record a zero descriptor size when desc is null. Co-Authored-By: Claude Opus 5.5 --- elfio/elfio_note.hpp | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/elfio/elfio_note.hpp b/elfio/elfio_note.hpp index 29fdb82df..c58235769 100644 --- a/elfio/elfio_note.hpp +++ b/elfio/elfio_note.hpp @@ -111,6 +111,11 @@ class note_section_accessor_template { const auto& convertor = elf_file.get_convertor(); + // A record without descriptor data must not claim a descriptor + if ( desc == nullptr ) { + descSize = 0; + } + int align = sizeof( Elf_Word ); Elf_Word nameLen = (Elf_Word)name.size() + 1; Elf_Word nameLenConv = ( *convertor )( nameLen ); From e81e4877ede2ab461f17825f6b2a478f631bda89 Mon Sep 17 00:00:00 2001 From: sstamenk Date: Tue, 13 Oct 2026 14:33:36 +0200 Subject: [PATCH 4/6] Keep notes with unterminated owner names and align added notes Some producers omit the owner name's terminator (e.g. the Freescale StarCore notes in tests/elf_examples/startup*.eln). Rejecting those records dropped the note and every note after it; return the full name instead. add_note() now pads existing data to a 4-byte boundary before appending, so a note added after a record with truncated padding still starts where a fresh accessor expects it. Co-Authored-By: Claude Opus 5.5 --- elfio/elfio_note.hpp | 14 ++++++++------ tests/ELFIONoteBoundsTest.cpp | 12 +++++++----- 2 files changed, 15 insertions(+), 11 deletions(-) diff --git a/elfio/elfio_note.hpp b/elfio/elfio_note.hpp index c58235769..4aaa8e5e3 100644 --- a/elfio/elfio_note.hpp +++ b/elfio/elfio_note.hpp @@ -138,8 +138,11 @@ class note_section_accessor_template } } - note_start_positions.emplace_back( ( notes->*F_get_size )() ); - notes->append_data( buffer ); + // Records start on a 4-byte boundary, even after data that does not + const Elf_Xword current_size = ( notes->*F_get_size )(); + const size_t padding = ( align - current_size % align ) % align; + note_start_positions.emplace_back( current_size + padding ); + notes->append_data( std::string( padding, '\0' ) + buffer ); } private: @@ -186,12 +189,11 @@ class note_section_accessor_template } const char* name = data + position + header_size; - if ( namesz != 0 && name[namesz - 1] != '\0' ) { - return false; - } + // Some producers omit the owner's terminator; keep the full name then + const bool terminated = namesz != 0 && name[namesz - 1] == '\0'; record.type = ( *convertor )( header[2] ); record.name = name; - record.name_size = namesz == 0 ? 0 : namesz - 1; + record.name_size = terminated ? namesz - 1 : namesz; record.descriptor = descsz == 0 ? nullptr : name + padded_name; record.descriptor_size = descsz; record.total_size = header_size + padded_name + padded_desc; diff --git a/tests/ELFIONoteBoundsTest.cpp b/tests/ELFIONoteBoundsTest.cpp index 201a726e3..aff057b8c 100644 --- a/tests/ELFIONoteBoundsTest.cpp +++ b/tests/ELFIONoteBoundsTest.cpp @@ -187,16 +187,18 @@ TEST_P( ELFIONoteBoundsTest, RevalidatesCachedOffsetsWithoutChangingOutputs ) expect_note( reader, 1, 2, "", "abc" ); } -TEST_P( ELFIONoteBoundsTest, RejectsUnterminatedOwnerName ) +TEST_P( ELFIONoteBoundsTest, KeepsUnterminatedOwnerName ) { - auto data = make_note( file, 7, std::string( "GNU\0", 4 ) ); + // Some producers omit the owner's terminator; the full name is returned. + auto data = make_note( file, 7, std::string( "GNU\0", 4 ), "abc" ); notes->set_data( data ); note_section_accessor cached( file, notes ); - data.back() = 'x'; + data[3 * sizeof( Elf_Word ) + 3] = 'x'; notes->set_data( data ); - expect_rejected( cached, 0 ); + expect_note( cached, 0, 7, "GNUx", "abc" ); note_section_accessor fresh( file, notes ); - EXPECT_EQ( fresh.get_notes_num(), 0U ); + ASSERT_EQ( fresh.get_notes_num(), 1U ); + expect_note( fresh, 0, 7, "GNUx", "abc" ); } TEST_P( ELFIONoteBoundsTest, ReadsSectionAndSegmentNotes ) From 2d2c73d8d4b86bbe89dc11d67faf3b2e8340baa4 Mon Sep 17 00:00:00 2001 From: sstamenk Date: Tue, 13 Oct 2026 14:55:07 +0200 Subject: [PATCH 5/6] Rescan notes after padding in add_note When add_note() pads data that ends mid-record, the padding can complete an earlier record that was rejected, so the calling accessor now rescans in that case and numbers notes the same way a fresh accessor does. The padding is computed in Elf_Xword to avoid sign and 32-bit narrowing conversions. Co-Authored-By: Claude Opus 5.5 --- elfio/elfio_note.hpp | 21 ++++++++++++++++----- 1 file changed, 16 insertions(+), 5 deletions(-) diff --git a/elfio/elfio_note.hpp b/elfio/elfio_note.hpp index 4aaa8e5e3..39c6cc7a0 100644 --- a/elfio/elfio_note.hpp +++ b/elfio/elfio_note.hpp @@ -138,11 +138,22 @@ class note_section_accessor_template } } - // Records start on a 4-byte boundary, even after data that does not - const Elf_Xword current_size = ( notes->*F_get_size )(); - const size_t padding = ( align - current_size % align ) % align; - note_start_positions.emplace_back( current_size + padding ); - notes->append_data( std::string( padding, '\0' ) + buffer ); + // Records start on a 4-byte boundary, even when the existing data + // does not end on one + constexpr Elf_Xword word_size = sizeof( Elf_Word ); + const Elf_Xword current_size = ( notes->*F_get_size )(); + const Elf_Xword padding = + ( word_size - current_size % word_size ) % word_size; + notes->append_data( + std::string( static_cast( padding ), '\0' ) + buffer ); + if ( padding == 0 ) { + note_start_positions.emplace_back( current_size ); + } + else { + // The padding may complete a record that was rejected before, so + // rescan to keep indices consistent with a fresh accessor + process_section(); + } } private: From 5aaee4fd66b904b7bbbc4709c785a1a2d78a38da Mon Sep 17 00:00:00 2001 From: sstamenk Date: Tue, 13 Oct 2026 15:08:27 +0200 Subject: [PATCH 6/6] Rescan notes whenever add_note follows undecoded data Rescanning only when padding was added missed an aligned section ending in an incomplete record: the new note's header could complete that record, leaving the calling accessor's indices different from a fresh accessor's. Track where the last scan stopped and append directly only when the new record starts there; otherwise rescan. Co-Authored-By: Claude Opus 5.5 --- elfio/elfio_note.hpp | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/elfio/elfio_note.hpp b/elfio/elfio_note.hpp index 39c6cc7a0..8161c9173 100644 --- a/elfio/elfio_note.hpp +++ b/elfio/elfio_note.hpp @@ -146,11 +146,14 @@ class note_section_accessor_template ( word_size - current_size % word_size ) % word_size; notes->append_data( std::string( static_cast( padding ), '\0' ) + buffer ); - if ( padding == 0 ) { + if ( current_size == scanned_size ) { + // The existing data decoded completely, so the new record simply + // follows it note_start_positions.emplace_back( current_size ); + scanned_size = current_size + buffer.size(); } else { - // The padding may complete a record that was rejected before, so + // The appended bytes may complete a record the scan stopped at, so // rescan to keep indices consistent with a fresh accessor process_section(); } @@ -216,6 +219,7 @@ class note_section_accessor_template void process_section() { note_start_positions.clear(); + scanned_size = 0; if ( notes == nullptr ) { return; } @@ -227,6 +231,7 @@ class note_section_accessor_template note_start_positions.emplace_back( current ); current += record.total_size; } + scanned_size = current; } //------------------------------------------------------------------------------ @@ -235,6 +240,7 @@ class note_section_accessor_template S* notes; //!< Pointer to the section or segment std::vector note_start_positions; //!< Vector of note start positions + Elf_Xword scanned_size = 0; //!< End of the records decoded by the last scan }; using note_section_accessor =