mime: address scan-build warnings

util-decode-mime.c:189:31: warning: Use of memory after it is freed [unix.Malloc]
            lastSibling->next = entity->child;
            ~~~~~~~~~~~~~~~~~ ^
util-decode-mime.c:827:24: warning: Potential leak of memory pointed to by 'val' [unix.Malloc]
        state->hname = NULL;
                       ^~~~
/usr/lib/llvm-16/lib/clang/16/include/stddef.h:89:24: note: expanded from macro 'NULL'
 #  define NULL ((void*)0)
                       ^
2 warnings generated.

Improve error handling and add assert to avoid these warnings.

Bug: #3147.
pull/8753/head
Victor Julien 2 years ago
parent b625aa9748
commit 9224b3435b

@ -180,27 +180,21 @@ void MimeDecFreeEntity (MimeDecEntity *entity)
MimeDecEntity *lastSibling = findLastSibling(entity); MimeDecEntity *lastSibling = findLastSibling(entity);
while (entity != NULL) while (entity != NULL)
{ {
/** /* move child to next to transform the tree into a list */
* Move child to next if (entity->child != NULL) {
* Transform tree into list
*/
if (entity->child != NULL)
{
lastSibling->next = entity->child; lastSibling->next = entity->child;
entity->child = NULL;
lastSibling = findLastSibling(lastSibling); lastSibling = findLastSibling(lastSibling);
} }
/** MimeDecEntity *next = entity->next;
* Move to next element DEBUG_VALIDATE_BUG_ON(
*/ (next != NULL && entity == lastSibling) || (next == NULL && entity != lastSibling));
MimeDecEntity *old = entity; MimeDecFreeField(entity->field_list);
entity = entity->next; MimeDecFreeUrl(entity->url_list);
SCFree(entity->filename);
MimeDecFreeField(old->field_list); SCFree(entity);
MimeDecFreeUrl(old->url_list); entity = next;
SCFree(old->filename);
SCFree(old);
} }
} }
@ -423,9 +417,13 @@ MimeDecEntity * MimeDecAddEntity(MimeDecEntity *parent)
* \param vlen Length of the value * \param vlen Length of the value
* *
* \return The field or NULL if the operation fails * \return The field or NULL if the operation fails
*
* name and val are passed as ptr to ptr and each will be set to NULL
* only if the pointer is consumed. This gives the caller an easy way
* to free the memory if not consumed.
*/ */
static MimeDecField * MimeDecFillField(MimeDecEntity *entity, uint8_t *name, static MimeDecField *MimeDecFillField(
uint32_t nlen, const uint8_t *value, uint32_t vlen) MimeDecEntity *entity, uint8_t **name, uint32_t nlen, uint8_t **value, uint32_t vlen)
{ {
if (nlen == 0 && vlen == 0) if (nlen == 0 && vlen == 0)
return NULL; return NULL;
@ -436,18 +434,21 @@ static MimeDecField * MimeDecFillField(MimeDecEntity *entity, uint8_t *name,
} }
if (nlen > 0) { if (nlen > 0) {
uint8_t *n = *name;
/* convert to lowercase and store */ /* convert to lowercase and store */
uint32_t u; for (uint32_t u = 0; u < nlen; u++)
for (u = 0; u < nlen; u++) n[u] = u8_tolower(n[u]);
name[u] = u8_tolower(name[u]);
field->name = (uint8_t *)name; field->name = (uint8_t *)n;
field->name_len = nlen; field->name_len = nlen;
*name = NULL;
} }
if (vlen > 0) { if (vlen > 0) {
field->value = (uint8_t *)value; field->value = (uint8_t *)*value;
field->value_len = vlen; field->value_len = vlen;
*value = NULL;
} }
return field; return field;
@ -789,41 +790,38 @@ static uint8_t * GetToken(uint8_t *buf, uint32_t blen, const char *delims,
*/ */
static int StoreMimeHeader(MimeDecParseState *state) static int StoreMimeHeader(MimeDecParseState *state)
{ {
int ret = MIME_DEC_OK, stored = 0; int ret = MIME_DEC_OK;
uint8_t *val;
uint32_t vlen;
/* Lets save the most recent header */ /* Lets save the most recent header */
if (state->hname != NULL || state->hvalue != NULL) { if (state->hname != NULL || state->hvalue != NULL) {
SCLogDebug("Storing last header"); SCLogDebug("Storing last header");
val = GetFullValue(state->hvalue, &vlen); uint32_t vlen;
uint8_t *val = GetFullValue(state->hvalue, &vlen);
if (val != NULL) { if (val != NULL) {
if (state->hname == NULL) { if (state->hname == NULL) {
SCLogDebug("Error: Invalid parser state - header value without" SCLogDebug("Error: Invalid parser state - header value without"
" name"); " name");
ret = MIME_DEC_ERR_PARSE; ret = MIME_DEC_ERR_PARSE;
} else if (state->stack->top != NULL) { } else if (state->stack->top != NULL) {
/* Store each header name and value */ /* Store each header name and value */
if (MimeDecFillField(state->stack->top->data, state->hname, state->hlen, val, if (MimeDecFillField(state->stack->top->data, &state->hname, state->hlen, &val,
vlen) == NULL) { vlen) == NULL) {
ret = MIME_DEC_ERR_MEM; ret = MIME_DEC_ERR_MEM;
} else {
stored = 1;
} }
} else { } else {
SCLogDebug("Error: Stack pointer missing"); SCLogDebug("Error: Stack pointer missing");
ret = MIME_DEC_ERR_DATA; ret = MIME_DEC_ERR_DATA;
} }
} else if (state->hvalue != NULL) { } else {
/* Memory allocation must have failed since val is NULL */ if (state->hvalue != NULL) {
ret = MIME_DEC_ERR_MEM; /* Memory allocation must have failed since val is NULL */
ret = MIME_DEC_ERR_MEM;
}
} }
/* Do cleanup here */ SCFree(val);
if (!stored) { SCFree(state->hname);
SCFree(state->hname);
SCFree(val);
}
state->hname = NULL; state->hname = NULL;
FreeDataValue(state->hvalue); FreeDataValue(state->hvalue);
state->hvalue = NULL; state->hvalue = NULL;

Loading…
Cancel
Save