Bug #6904: mime: buffer overflow in GetFullValue() (util-decode-mime.c) - Suricata - Open Information Security Foundation

Actions

Copy link

Bug #6904

closed

mime: buffer overflow in GetFullValue() (util-decode-mime.c)

Added by Victor Julien 8 months ago. Updated 7 months ago.

Status:

Closed

Priority:

Normal

Assignee:

Philippe Antoine

Target version:

8.0.0-beta1

Affected Versions:

Effort:

Difficulty:

Label:

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

Actions

Copy link

Updated by Philippe Antoine 8 months ago

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 )

Actions

Copy link

Updated by Victor Julien 8 months ago

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.

Actions

Copy link

Updated by Philippe Antoine 8 months ago

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

Actions

Copy link