diff --git a/elfio/elfio_note.hpp b/elfio/elfio_note.hpp index c246fb527..8161c9173 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,36 +78,23 @@ 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 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* 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; } - name.assign( pData + 3 * (size_t)align, namesz - 1 ); - if ( 0 == descSize ) { - desc = nullptr; - } - else { - desc = const_cast( pData + 3 * (size_t)align + - ( ( namesz + align - 1 ) / align ) * - (size_t)align ); - } + name.assign( record.name, record.name_size ); + type = record.type; + descSize = record.descriptor_size; + desc = const_cast( record.descriptor ); return true; } @@ -122,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 ); @@ -144,46 +138,100 @@ 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 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 ( 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 appended bytes may complete a record the scan stopped at, so + // rescan to keep indices consistent with a fresh accessor + process_section(); + } } 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; + // 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 = 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; + 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 ) { + scanned_size = 0; + if ( notes == nullptr ) { 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 { - break; - } - - current += advance; + 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 += record.total_size; } + scanned_size = current; } //------------------------------------------------------------------------------ @@ -192,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 = diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index 1c5928531..841bdf62d 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -62,6 +62,7 @@ configure_file(runELFtests ${CMAKE_CURRENT_BINARY_DIR}/runELFtests COPYONLY) add_executable( ELFIOTest + ELFIONoteBoundsTest.cpp ELFIOTest.cpp ELFIOTest1.cpp ELFIOTest2.cpp diff --git a/tests/ELFIONoteBoundsTest.cpp b/tests/ELFIONoteBoundsTest.cpp new file mode 100644 index 000000000..aff057b8c --- /dev/null +++ b/tests/ELFIONoteBoundsTest.cpp @@ -0,0 +1,250 @@ +/* +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 + +#include +#include + +using namespace ELFIO; + +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 ) +{ + 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 ); +} + +template +void expect_note( const Accessor& reader, + Elf_Word index, + Elf_Word expected_type, + const std::string& expected_name, + const std::string& expected_descriptor ) +{ + 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 ); + } +} + +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() ); +} + +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_P( ELFIONoteBoundsTest, RejectsOversizedLengths ) +{ + 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 ); + } +} + +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, KeepsUnterminatedOwnerName ) +{ + // 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[3 * sizeof( Elf_Word ) + 3] = 'x'; + notes->set_data( data ); + expect_note( cached, 0, 7, "GNUx", "abc" ); + note_section_accessor fresh( file, notes ); + ASSERT_EQ( fresh.get_notes_num(), 1U ); + expect_note( fresh, 0, 7, "GNUx", "abc" ); +} + +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 ); +} + +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