Stowage/clamav
2
0
Fork 0
mirror of https://github.com/Cisco-Talos/clamav.git synced 2025-10-19 10:23:17 +00:00
Code Issues Projects Releases Packages Wiki Activity

Fix possible double-free in OLE2 document parser

Browse source 
If realloc() is called with size == 0, realloc() will free the pointer
and return NULL. Unless you check for this and set the pointer to NULL,
the pointer may later be free'd again after the `done:` label.

This commit fixes it by using the new CLI_REALLOC macro. CLI_REALLOC
uses `cli_realloc()` that both limits the amoutn of memory that may be
allocated and also will return NULL if you try to set size == 0, WITHOUT
free'ing the memory.

Resolves: https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=44040
This commit is contained in:
Micah Snyder 2022-01-25 16:12:41 -08:00
parent 1bfb0522cf
commit d1746ba0a5
2 changed files with 58 additions and 27 deletions
Download patch file Download diff file Expand all files Collapse all files

53
libclamav/others.h
View file

@ -919,8 +919,36 @@ static inline int cli_getpagesize(void)
void *cli_malloc(size_t nmemb);
void *cli_calloc(size_t nmemb, size_t size);
/**
* @brief Wrapper around realloc that limits how much may be allocated to CLI_MAX_ALLOCATION.
*
* Please use CLI_REALLOC() with `goto done;` error handling instead.
*
* IMPORTANT: This differs from realloc() in that if size==0, it will NOT free the ptr.
*
* @param ptr
* @param size
* @return void*
*/
void *cli_realloc(void *ptr, size_t size);
/**
* @brief Wrapper around realloc that limits how much may be allocated to CLI_MAX_ALLOCATION.
*
* Please use CLI_REALLOC() with `goto done;` error handling instead.
*
* IMPORTANT: This differs from realloc() in that if size==0, it will NOT free the ptr.
*
* WARNING: This differs from cli_realloc() in that it will free the ptr if the allocation fails.
* If you're using `goto done;` error handling, this may result in a double-free!!
*
* @param ptr
* @param size
* @return void*
*/
void *cli_realloc2(void *ptr, size_t size);
char *cli_strdup(const char *s);
int cli_rmdirs(const char *dirname);
char *cli_hashstream(FILE *fs, unsigned char *digcpy, int type);
@ -1273,4 +1301,29 @@ uint8_t cli_set_debug_flag(uint8_t debug_flag);
} while (0)
#endif
/**
* @brief Wrapper around realloc that limits how much may be allocated to CLI_MAX_ALLOCATION.
*
* IMPORTANT: This differs from realloc() in that if size==0, it will NOT free the ptr.
*
* NOTE: cli_realloc() will NOT free var if size==0. It is safe to free var after `done:`.
*
* @param ptr
* @param size
* @return void*
*/
#ifndef CLI_REALLOC
#define CLI_REALLOC(var, size, ...) \
do { \
void *vTmp = cli_realloc(var, size); \
if (NULL == vTmp) { \
do { \
__VA_ARGS__; \
} while (0); \
goto done; \
} \
var = vTmp; \
} while (0)
#endif
#endif

32
libclamav/xlm_extract.c
View file

@ -4816,15 +4816,8 @@ cl_error_t cli_extract_xlm_macros_and_images(const char *dir, cli_ctx *ctx, char
} else {
/* already found the beginning of a drawing group, extract the remaining chunks */
unsigned char *tmp = NULL;
drawinggroup_len += biff_header.length;
tmp = realloc(drawinggroup, drawinggroup_len);
if (NULL == tmp) {
cli_dbgmsg("Failed to allocate %zu bytes for extracted image\n", drawinggroup_len);
status = CL_EMEM;
goto done;
}
drawinggroup = tmp;
CLI_REALLOC(drawinggroup, drawinggroup_len, status = CL_EMEM);
memcpy(drawinggroup + (drawinggroup_len - biff_header.length), data, biff_header.length);
// cli_dbgmsg("Collected %d drawing group bytes\n", biff_header.length);
}
@ -4834,15 +4827,8 @@ cl_error_t cli_extract_xlm_macros_and_images(const char *dir, cli_ctx *ctx, char
if ((OPC_MSODRAWINGGROUP == previous_biff8_opcode) &&
(NULL != drawinggroup)) {
/* already found the beginning of an image, extract the remaining chunks */
unsigned char *tmp = NULL;
drawinggroup_len += biff_header.length;
tmp = realloc(drawinggroup, drawinggroup_len);
if (NULL == tmp) {
cli_dbgmsg("Failed to allocate %zu bytes for extracted image\n", drawinggroup_len);
status = CL_EMEM;
goto done;
}
drawinggroup = tmp;
CLI_REALLOC(drawinggroup, drawinggroup_len, status = CL_EMEM);
memcpy(drawinggroup + (drawinggroup_len - biff_header.length), data, biff_header.length);
// cli_dbgmsg("Collected %d image bytes\n", biff_header.length);
}
@ -5017,9 +5003,7 @@ cl_error_t cli_extract_xlm_macros_and_images(const char *dir, cli_ctx *ctx, char
status = CL_SUCCESS;
done:
if (NULL != drawinggroup) {
free(drawinggroup);
}
FREE(drawinggroup);
if (in_fd != -1) {
close(in_fd);
@ -5034,15 +5018,9 @@ done:
out_fd = -1;
}
if (data != NULL) {
free(data);
data = NULL;
}
FREE(data);
if (tempfile != NULL) {
free(tempfile);
tempfile = NULL;
}
FREE(tempfile);
return status;
}