Bug #6904
closed
mime: buffer overflow in GetFullValue() (util-decode-mime.c)
Added by Victor Julien 8 months ago.
Updated 7 months ago.
Description
static uint8_t *GetFullValue(const DataValue *dv, uint32_t *olen)
{
uint32_t offset = 0;
uint8_t *val = NULL;
uint32_t len = 0;
*olen = 0;
/* First calculate total length */
for (const DataValue *curr = dv; curr != NULL; curr = curr->next) {
[1] len += curr->value_len;
}
/* Must have at least one character in the value */
if (len > 0) {
[2] val = SCCalloc(1, len);
if (unlikely(val == NULL)) {
return NULL;
}
for (const DataValue *curr = dv; curr != NULL; curr = curr->next) {
[3] memcpy(val + offset, curr->value, curr->value_len);
offset += curr->value_len;
}
}
*olen = len;
return val;
}
1 - integer overflow is possible on this line
2 - when 'len' variable overflows, buffer of small size will be allocated
3 - heap overflow on this line
I agree that an unsigned integer overflow would lead to a buffer overflow.
This is a uint32_t
Even if there is no buffer overflow, it would be a security issue to try to allocate more than 4Gbytes in one go...
But this is not a problem because the callers ensure that the sum of DataValue
value_len fields is bounded by mdcfg->header_value_depth
(default 2000, cannot be unlimited)
The hypothesis
1 - integer overflow is possible on this line
looks wrong to me.
So, I do not think there is much to do (besides #3487 )
Is there a way that multiple lines could be combined to one? Together perhaps getting to the overflow size? Some protocols have continuation line logic, I believe SMTP has it as well. Not sure if MIME has.
Victor Julien wrote in #note-2:
Is there a way that multiple lines could be combined to one? Together perhaps getting to the overflow size? Some protocols have continuation line logic, I believe SMTP has it as well. Not sure if MIME has.
GetFullValue
combines indeed multiple lines into one header
but these lines/DataValue are only added only until the bound of mdcfg->header_value_depth is reached
- Target version changed from TBD to 8.0.0-beta1
Do we have a way to answer the reporters of this issue ?
- Status changed from New to In Review
- Status changed from In Review to Closed
- Tracker changed from Security to Bug
- Priority changed from High to Normal
- Severity deleted (
MODERATE)
- Private changed from Yes to No
Also available in: Atom
PDF