Skip to content

Commit 52f21a1

Browse files
committed
Fix potential overflow issues in ELFIO string and section handling
- Use Elf_Xword for section sizes - Updated `add_string` in [`string_section_accessor_template`](elfio/elfio_strings.hpp) to handle string length and section size overflow cases. - Enhanced `insert_data` and `append_data` in [`section_impl`](elfio/elfio_section.hpp) to check for size overflows before modifying section data.
1 parent 34d2c64 commit 52f21a1

File tree

4 files changed

+154
-48
lines changed

4 files changed

+154
-48
lines changed

elfio/elfio_dynamic.hpp

Lines changed: 16 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -150,17 +150,29 @@ template <class S> class dynamic_section_accessor_template
150150

151151
// Check unusual case when dynamic section has no data
152152
if ( dynamic_section->get_data() == nullptr ||
153-
( index + 1 ) * dynamic_section->get_entry_size() >
154-
dynamic_section->get_size() ||
155153
dynamic_section->get_entry_size() < sizeof( T ) ) {
156154
tag = DT_NULL;
157155
value = 0;
158156
return;
159157
}
160158

159+
// Check for integer overflow in size calculation
160+
if (index > (dynamic_section->get_size() / dynamic_section->get_entry_size()) - 1) {
161+
tag = DT_NULL;
162+
value = 0;
163+
return;
164+
}
165+
166+
// Check for integer overflow in pointer arithmetic
167+
Elf_Xword offset = index * dynamic_section->get_entry_size();
168+
if (offset > dynamic_section->get_size() - sizeof(T)) {
169+
tag = DT_NULL;
170+
value = 0;
171+
return;
172+
}
173+
161174
const T* pEntry = reinterpret_cast<const T*>(
162-
dynamic_section->get_data() +
163-
index * dynamic_section->get_entry_size() );
175+
dynamic_section->get_data() + offset );
164176
tag = convertor( pEntry->d_tag );
165177
switch ( tag ) {
166178
case DT_NULL:

elfio/elfio_section.hpp

Lines changed: 82 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -70,7 +70,7 @@ class section
7070
* @param raw_data Pointer to the raw data.
7171
* @param size Size of the data.
7272
*/
73-
virtual void set_data( const char* raw_data, Elf_Word size ) = 0;
73+
virtual void set_data( const char* raw_data, Elf_Xword size ) = 0;
7474

7575
/**
7676
* @brief Set the data of the section.
@@ -83,7 +83,7 @@ class section
8383
* @param raw_data Pointer to the raw data.
8484
* @param size Size of the data.
8585
*/
86-
virtual void append_data( const char* raw_data, Elf_Word size ) = 0;
86+
virtual void append_data( const char* raw_data, Elf_Xword size ) = 0;
8787

8888
/**
8989
* @brief Append data to the section.
@@ -98,7 +98,7 @@ class section
9898
* @param size Size of the data.
9999
*/
100100
virtual void
101-
insert_data( Elf_Xword pos, const char* raw_data, Elf_Word size ) = 0;
101+
insert_data( Elf_Xword pos, const char* raw_data, Elf_Xword size ) = 0;
102102

103103
/**
104104
* @brief Insert data into the section at a specific position.
@@ -250,10 +250,10 @@ template <class T> class section_impl : public section
250250
* @param raw_data Pointer to the raw data.
251251
* @param size Size of the data.
252252
*/
253-
void set_data( const char* raw_data, Elf_Word size ) override
253+
void set_data( const char* raw_data, Elf_Xword size ) override
254254
{
255255
if ( get_type() != SHT_NOBITS ) {
256-
data = std::unique_ptr<char[]>( new ( std::nothrow ) char[size] );
256+
data = std::unique_ptr<char[]>( new ( std::nothrow ) char[(size_t)size] );
257257
if ( nullptr != data.get() && nullptr != raw_data ) {
258258
data_size = size;
259259
std::copy( raw_data, raw_data + size, data.get() );
@@ -265,7 +265,7 @@ template <class T> class section_impl : public section
265265

266266
set_size( data_size );
267267
if ( translator->empty() ) {
268-
set_stream_size( data_size );
268+
set_stream_size( (size_t)data_size );
269269
}
270270
}
271271

@@ -283,7 +283,7 @@ template <class T> class section_impl : public section
283283
* @param raw_data Pointer to the raw data.
284284
* @param size Size of the data.
285285
*/
286-
void append_data( const char* raw_data, Elf_Word size ) override
286+
void append_data( const char* raw_data, Elf_Xword size ) override
287287
{
288288
insert_data( get_size(), raw_data, size );
289289
}
@@ -304,19 +304,46 @@ template <class T> class section_impl : public section
304304
* @param size Size of the data.
305305
*/
306306
void
307-
insert_data( Elf_Xword pos, const char* raw_data, Elf_Word size ) override
307+
insert_data( Elf_Xword pos, const char* raw_data, Elf_Xword size ) override
308308
{
309309
if ( get_type() != SHT_NOBITS ) {
310-
if ( get_size() + size < data_size ) {
310+
// Check for valid position
311+
if ( pos > get_size() ) {
312+
return; // Invalid position
313+
}
314+
315+
// Check for integer overflow in size calculation
316+
Elf_Xword new_size = get_size();
317+
if ( size > std::numeric_limits<Elf_Xword>::max() - new_size ) {
318+
return; // Size would overflow
319+
}
320+
new_size += size;
321+
322+
if ( new_size <= data_size ) {
311323
char* d = data.get();
312324
std::copy_backward( d + pos, d + get_size(),
313325
d + get_size() + size );
314326
std::copy( raw_data, raw_data + size, d + pos );
315327
}
316328
else {
317-
data_size = 2 * ( data_size + size );
329+
// Calculate new size with overflow check
330+
Elf_Xword new_data_size = data_size;
331+
if ( new_data_size > std::numeric_limits<Elf_Xword>::max() / 2 ) {
332+
return; // Multiplication would overflow
333+
}
334+
new_data_size *= 2;
335+
if ( size > std::numeric_limits<Elf_Xword>::max() - new_data_size ) {
336+
return; // Addition would overflow
337+
}
338+
new_data_size += size;
339+
340+
// Check if the size would overflow size_t
341+
if (new_data_size > std::numeric_limits<size_t>::max()) {
342+
return; // Size would overflow size_t
343+
}
344+
318345
std::unique_ptr<char[]> new_data(
319-
new ( std::nothrow ) char[data_size] );
346+
new (std::nothrow) char[(size_t)new_data_size]);
320347

321348
if ( nullptr != new_data ) {
322349
char* d = data.get();
@@ -326,14 +353,15 @@ template <class T> class section_impl : public section
326353
std::copy( d + pos, d + get_size(),
327354
new_data.get() + pos + size );
328355
data = std::move( new_data );
356+
data_size = new_data_size;
329357
}
330358
else {
331-
size = 0;
359+
return; // Allocation failed
332360
}
333361
}
334-
set_size( get_size() + size );
362+
set_size( new_size );
335363
if ( translator->empty() ) {
336-
set_stream_size( get_stream_size() + size );
364+
set_stream_size( get_stream_size() + (size_t)size );
337365
}
338366
}
339367
}
@@ -431,36 +459,55 @@ template <class T> class section_impl : public section
431459
*/
432460
bool load_data() const
433461
{
434-
Elf_Xword sh_offset =
435-
( *translator )[( *convertor )( header.sh_offset )];
462+
Elf_Xword sh_offset = (*translator)[(*convertor)(header.sh_offset)];
436463
Elf_Xword size = get_size();
437-
if ( nullptr == data && SHT_NULL != get_type() &&
438-
SHT_NOBITS != get_type() && sh_offset <= get_stream_size() &&
439-
size <= ( get_stream_size() - sh_offset ) ) {
440-
data.reset( new ( std::nothrow ) char[size_t( size ) + 1] );
441-
442-
if ( ( 0 != size ) && ( nullptr != data ) ) {
443-
pstream->seekg( sh_offset );
444-
pstream->read( data.get(), size );
445-
if ( static_cast<Elf_Xword>( pstream->gcount() ) != size ) {
446-
data = nullptr;
464+
465+
// Check for integer overflow in offset calculation
466+
if (sh_offset > get_stream_size()) {
467+
return false;
468+
}
469+
470+
// Check for integer overflow in size calculation
471+
if (size > get_stream_size() ||
472+
size > (get_stream_size() - sh_offset)) {
473+
return false;
474+
}
475+
476+
// Check if we need to load data
477+
if (nullptr == data && SHT_NULL != get_type() && SHT_NOBITS != get_type()) {
478+
// Check if size can be safely converted to size_t
479+
if (size > std::numeric_limits<size_t>::max() - 1) {
480+
return false;
481+
}
482+
483+
data.reset(new (std::nothrow) char[size_t(size) + 1]);
484+
485+
if ((0 != size) && (nullptr != data)) {
486+
pstream->seekg(sh_offset);
487+
pstream->read(data.get(), size);
488+
if (static_cast<Elf_Xword>(pstream->gcount()) != size) {
489+
data.reset(nullptr);
490+
data_size = 0;
447491
return false;
448492
}
449493

450-
// refresh size because it may have changed if we had to decompress data
451-
size = get_size();
452-
data.get()[size] =
453-
0; // Ensure data is ended with 0 to avoid oob read
454-
data_size = decltype( data_size )( size );
494+
data_size = size;
495+
data.get()[size] = 0; // Safe now as we allocated size + 1
455496
}
456497
else {
457498
data_size = 0;
499+
if (size != 0) {
500+
return false; // Failed to allocate required memory
501+
}
458502
}
503+
504+
is_loaded = true;
505+
return true;
459506
}
460507

461-
is_loaded = true;
462-
463-
return true;
508+
// Data already loaded or doesn't need loading
509+
is_loaded = (nullptr != data) || (SHT_NULL == get_type()) || (SHT_NOBITS == get_type());
510+
return is_loaded;
464511
}
465512

466513
/**
@@ -528,7 +575,7 @@ template <class T> class section_impl : public section
528575
Elf_Half index = 0; /**< Index of the section. */
529576
std::string name; /**< Name of the section. */
530577
mutable std::unique_ptr<char[]> data; /**< Pointer to the data. */
531-
mutable Elf_Word data_size = 0; /**< Size of the data. */
578+
mutable Elf_Xword data_size = 0; /**< Size of the data. */
532579
const endianness_convertor* convertor =
533580
nullptr; /**< Pointer to the endianness convertor. */
534581
const address_translator* translator =

elfio/elfio_segment.hpp

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -299,6 +299,7 @@ template <class T> class segment_impl : public segment
299299

300300
stream.seekg( ( *translator )[header_offset] );
301301
stream.read( reinterpret_cast<char*>( &ph ), sizeof( ph ) );
302+
302303
is_offset_set = true;
303304

304305
if ( !( is_lazy || is_loaded ) ) {
@@ -320,7 +321,21 @@ template <class T> class segment_impl : public segment
320321
Elf_Xword p_offset = ( *translator )[( *convertor )( ph.p_offset )];
321322
Elf_Xword size = get_file_size();
322323

323-
if ( p_offset + size > get_stream_size() ) {
324+
// Check for integer overflow in offset calculation
325+
if (p_offset > get_stream_size()) {
326+
data = nullptr;
327+
return false;
328+
}
329+
330+
// Check for integer overflow in size calculation
331+
if (size > get_stream_size() ||
332+
size > (get_stream_size() - p_offset)) {
333+
data = nullptr;
334+
return false;
335+
}
336+
337+
// Check if size can be safely converted to size_t
338+
if (size > std::numeric_limits<size_t>::max() - 1) {
324339
data = nullptr;
325340
return false;
326341
}

elfio/elfio_strings.hpp

Lines changed: 40 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ THE SOFTWARE.
2626
#include <cstdlib>
2727
#include <cstring>
2828
#include <string>
29+
#include <limits>
2930

3031
namespace ELFIO {
3132

@@ -51,12 +52,26 @@ template <class S> class string_section_accessor_template
5152
{
5253
if ( string_section ) {
5354
const char* data = string_section->get_data();
54-
if ( index < string_section->get_size() && nullptr != data ) {
55-
size_t string_length = strnlength(
56-
data + index,
57-
static_cast<size_t>( string_section->get_size() ) - index );
58-
if ( string_length < ( string_section->get_size() - index ) )
59-
return data + index;
55+
size_t section_size =
56+
static_cast<size_t>( string_section->get_size() );
57+
58+
// Check if index is within bounds
59+
if ( index >= section_size || nullptr == data ) {
60+
return nullptr;
61+
}
62+
63+
// Check for integer overflow in size calculation
64+
size_t remaining_size = section_size - index;
65+
if ( remaining_size > section_size ) { // Check for underflow
66+
return nullptr;
67+
}
68+
69+
// Use standard C++ functions to find string length
70+
const char* str = data + index;
71+
const char* end =
72+
(const char*)std::memchr( str, '\0', remaining_size );
73+
if ( end != nullptr && end < str + remaining_size ) {
74+
return str;
6075
}
6176
}
6277

@@ -69,6 +84,10 @@ template <class S> class string_section_accessor_template
6984
//! \return Index of the added string
7085
Elf_Word add_string( const char* str )
7186
{
87+
if ( !str ) {
88+
return 0; // Return index of empty string for null input
89+
}
90+
7291
Elf_Word current_position = 0;
7392

7493
if ( string_section ) {
@@ -81,8 +100,21 @@ template <class S> class string_section_accessor_template
81100
string_section->append_data( &empty_string, 1 );
82101
current_position++;
83102
}
84-
string_section->append_data(
85-
str, static_cast<Elf_Word>( std::strlen( str ) + 1 ) );
103+
104+
// Calculate string length and check for overflow
105+
size_t str_len = std::strlen( str );
106+
if ( str_len > std::numeric_limits<Elf_Word>::max() - 1 ) {
107+
return 0; // String too long
108+
}
109+
110+
// Check if appending would overflow section size
111+
Elf_Word append_size = static_cast<Elf_Word>( str_len + 1 );
112+
if ( append_size >
113+
std::numeric_limits<Elf_Word>::max() - current_position ) {
114+
return 0; // Would overflow section size
115+
}
116+
117+
string_section->append_data( str, append_size );
86118
}
87119

88120
return current_position;

0 commit comments

Comments
 (0)