Description
Hey
Inside parseString
, the arguments data
, and num_components
are entirely user controlled. They are added together with base
and used to check if we can safely read the contents of our string inside our given len
. The problem here is that both data
and num_components
are unsigned int
. This means if the user provides a big enough value for the first check, we can overflow the calculation:
static std::string parseString(const uint8_t* buf,
unsigned num_components,
unsigned data,
unsigned base,
unsigned len,
bool intel)
{
std::string value;
if (num_components <= 4) {
// ...
} else
// [1] Calculation here, data = 0xffffffff, num_components < len
if (base+data+num_components <= len) {
const char* const sz((const char*)buf+base+data);
unsigned num(0);
// [2] segfault when we check `sz[num] != '\0'`
while (num < num_components && sz[num] != '\0')
++num;
while (num && sz[num-1] == ' ')
--num;
// Copy `num` chars from `sz` into `value`.
value.assign(sz, num);
}
return value;
}
};
If, for example we set data
as 0xffffffff
, and then num_components
as a small value, the calculation will overflow and the result can be less than len
. Then when we pass this check, we add data
to our buf
pointer. In this case, sz
will now be pointing to unmapped memory owing to the addition of 0xffffffff
, and will cause a segfault when we try to dereference to check for null bytes.
To mitigate this, perhaps you could add a small check before [1]
to ensure it doesnt overflow.
Note that this doesnt have to be a segfault. With smaller values, this will alow us to read out of bounds on the heap, reading contents from other chunks and copying them into value
.
Thanks for your time.