Skip to content

Commit

Permalink
Refine max-allocation and safer-allocation function and macro names
Browse files Browse the repository at this point in the history
We add the _OR_GOTO_DONE suffix to the macros that go to done if the
allocation fails. This makes it obvious what is different about the
macro versus the equivalent function, and that error handling is
built-in.

Renamed the cli_strdup to safer_strdup to make it obvious that it exists
because it is safer than regular strdup. Regular strdup doesn't have the
NULL check before trying to dup, and so may result in a NULL-deref
crash.

Also remove unused STRDUP (_OR_GOTO_DONE) macro, since the one with the
NULL-check is preferred.
  • Loading branch information
micahsnyder committed Mar 15, 2024
1 parent 39070d1 commit 405829e
Show file tree
Hide file tree
Showing 44 changed files with 409 additions and 358 deletions.
2 changes: 1 addition & 1 deletion clamdscan/client.c
Original file line number Diff line number Diff line change
Expand Up @@ -188,7 +188,7 @@ int16_t ping_clamd(const struct optstruct *opts)
/* ping command takes the form --ping [attempts[:interval]] */
if (NULL != (opt = optget(opts, "ping"))) {
if (NULL != opt->strarg) {
if (NULL == (attempt_str = cli_strdup(opt->strarg))) {
if (NULL == (attempt_str = cli_safer_strdup(opt->strarg))) {
logg(LOGG_ERROR, "could not allocate memory for string\n");
ret = -1;
goto done;
Expand Down
2 changes: 1 addition & 1 deletion clamonacc/client/client.c
Original file line number Diff line number Diff line change
Expand Up @@ -208,7 +208,7 @@ int16_t onas_ping_clamd(struct onas_context **ctx)
opt = optget((*ctx)->opts, "ping");

if (opt->enabled) {
attempt_str = cli_strdup(opt->strarg);
attempt_str = cli_safer_strdup(opt->strarg);
if (attempt_str) {
if (NULL == attempt_str) {
logg(LOGG_ERROR, "could not allocate memory for string\n");
Expand Down
2 changes: 1 addition & 1 deletion clamonacc/fanotif/fanotif.c
Original file line number Diff line number Diff line change
Expand Up @@ -255,7 +255,7 @@ int onas_fan_eloop(struct onas_context **ctx)
return 2;
}
memcpy(event_data->fmd, fmd, sizeof(struct fanotify_event_metadata));
event_data->pathname = cli_strdup(fname);
event_data->pathname = cli_safer_strdup(fname);
if (NULL == event_data->pathname) {
close(fmd->fd);
free(event_data->fmd);
Expand Down
2 changes: 1 addition & 1 deletion clamonacc/inotif/inotif.c
Original file line number Diff line number Diff line change
Expand Up @@ -821,7 +821,7 @@ static void onas_ddd_handle_extra_scanning(struct onas_context *ctx, const char

/* general mapping */
onas_map_context_info_to_event_data(ctx, &event_data);
event_data->pathname = cli_strdup(pathname);
event_data->pathname = cli_safer_strdup(pathname);
event_data->bool_opts |= ONAS_SCTH_B_SCAN;

/* inotify specific stuffs */
Expand Down
8 changes: 4 additions & 4 deletions freshclam/freshclam.c
Original file line number Diff line number Diff line change
Expand Up @@ -480,7 +480,7 @@ static fc_error_t get_server_node(
* Ensure that URL contains protocol.
*/
if (!strncmp(server, "db.", 3) && strstr(server, ".clamav.net")) {
url = cli_strdup("https://database.clamav.net");
url = cli_safer_strdup("https://database.clamav.net");
if (NULL == url) {
logg(LOGG_ERROR, "get_server_node: Failed to duplicate string for database.clamav.net url.\n");
status = FC_EMEM;
Expand All @@ -497,7 +497,7 @@ static fc_error_t get_server_node(
snprintf(url, urlLen + 1, "%s://%s", defaultProtocol, server);
} else {
urlLen = strlen(server);
url = cli_strdup(server);
url = cli_safer_strdup(server);
if (NULL == url) {
logg(LOGG_ERROR, "get_server_node: Failed to duplicate string for server url.\n");
status = FC_EMEM;
Expand Down Expand Up @@ -544,7 +544,7 @@ static fc_error_t string_list_add(const char *item, char ***stringList, uint32_t

*stringList = newList;

newList[nItems - 1] = cli_strdup(item);
newList[nItems - 1] = cli_safer_strdup(item);
if (newList[nItems - 1] == NULL) {
mprintf(LOGG_ERROR, "string_list_add: Failed to allocate memory for optional database list item.\n");
status = FC_EMEM;
Expand Down Expand Up @@ -1663,7 +1663,7 @@ int main(int argc, char **argv)
/*
* Parse the config file.
*/
cfgfile = cli_strdup(optget(opts, "config-file")->strarg);
cfgfile = cli_safer_strdup(optget(opts, "config-file")->strarg);
if ((opts = optparse(cfgfile, 0, NULL, 1, OPT_FRESHCLAM, 0, opts)) == NULL) {
fprintf(stderr, "ERROR: Can't open/parse the config file %s\n", cfgfile);
status = FC_EINIT;
Expand Down
4 changes: 2 additions & 2 deletions libclamav/blob.c
Original file line number Diff line number Diff line change
Expand Up @@ -154,7 +154,7 @@ void blobSetFilename(blob *b, const char *dir, const char *filename)
if (b->name)
free(b->name);

b->name = cli_strdup(filename);
b->name = cli_safer_strdup(filename);

if (b->name)
sanitiseName(b->name);
Expand Down Expand Up @@ -526,7 +526,7 @@ void fileblobPartialSet(fileblob *fb, const char *fullname, const char *arg)
fb->b.len = fb->b.size = 0;
fb->isNotEmpty = 1;
}
fb->fullname = cli_strdup(fullname);
fb->fullname = cli_safer_strdup(fullname);
}

void fileblobSetFilename(fileblob *fb, const char *dir, const char *filename)
Expand Down
8 changes: 4 additions & 4 deletions libclamav/bytecode.c
Original file line number Diff line number Diff line change
Expand Up @@ -507,7 +507,7 @@ static inline operand_t readOperand(struct cli_bc_func *func, unsigned char *p,
uint16_t ty;
p[*off] |= 0x20;
/* TODO: unique constants */
func->constants = cli_safer_realloc2(func->constants, (func->numConstants + 1) * sizeof(*func->constants));
func->constants = cli_safer_realloc_or_free(func->constants, (func->numConstants + 1) * sizeof(*func->constants));
if (!func->constants) {
*ok = false;
return MAX_OP;
Expand Down Expand Up @@ -698,13 +698,13 @@ static cl_error_t parseLSig(struct cli_bc *bc, char *buffer)
// char *vnames;
char *vend = strchr(buffer, ';');
if (vend) {
bc->lsig = cli_strdup(buffer);
bc->lsig = cli_safer_strdup(buffer);
*vend++ = '\0';
// prefix = buffer;
// vnames = strchr(vend, '{');
} else {
/* Not a logical signature, but we still have a virusname */
bc->hook_name = cli_strdup(buffer);
bc->hook_name = cli_safer_strdup(buffer);
bc->lsig = NULL;
}

Expand Down Expand Up @@ -2407,7 +2407,7 @@ static cl_error_t add_selfcheck(struct cli_all_bc *bcs)
struct cli_bc_inst *inst;
struct cli_bc *bc;

bcs->all_bcs = cli_safer_realloc2(bcs->all_bcs, sizeof(*bcs->all_bcs) * (bcs->count + 1));
bcs->all_bcs = cli_safer_realloc_or_free(bcs->all_bcs, sizeof(*bcs->all_bcs) * (bcs->count + 1));
if (!bcs->all_bcs) {
cli_errmsg("cli_loadcbc: Can't allocate memory for bytecode entry\n");
return CL_EMEM;
Expand Down
2 changes: 1 addition & 1 deletion libclamav/crtmgr.c
Original file line number Diff line number Diff line change
Expand Up @@ -415,7 +415,7 @@ static cl_error_t crtmgr_get_recov_data(BIGNUM *sig, cli_crt *x509,
if (!x)
goto done;

MALLOC(d, keylen);
CLI_MALLOC_OR_GOTO_DONE(d, keylen);

if (!BN_mod_exp(x, sig, x509->e, x509->n, bnctx)) {
cli_warnmsg("crtmgr_rsa_verify: verification failed: BN_mod_exp failed.\n");
Expand Down
2 changes: 1 addition & 1 deletion libclamav/cvd.c
Original file line number Diff line number Diff line change
Expand Up @@ -633,7 +633,7 @@ cl_error_t cli_cvdload(FILE *fs, struct cl_engine *engine, unsigned int *signo,

if (dbtype <= 1) {
/* check for duplicate db */
dupname = cli_strdup(filename);
dupname = cli_safer_strdup(filename);
if (!dupname)
return CL_EMEM;
dupname[strlen(dupname) - 2] = (dbtype == 1 ? 'v' : 'l');
Expand Down
24 changes: 12 additions & 12 deletions libclamav/egg.c
Original file line number Diff line number Diff line change
Expand Up @@ -1221,7 +1221,7 @@ static cl_error_t egg_parse_file_extra_field(egg_handle* handle, egg_file* eggFi
/*
* Comment found. Add comment to our list.
*/
CLI_SAFER_REALLOC(eggFile->comments,
CLI_SAFER_REALLOC_OR_GOTO_DONE(eggFile->comments,
sizeof(char*) * (eggFile->nComments + 1),
free(comment),
status = CL_EMEM);
Expand Down Expand Up @@ -1667,7 +1667,7 @@ cl_error_t cli_egg_open(fmap_t* map, void** hArchive, char*** comments, uint32_t
goto done;
} else {
/* Add file to list. */
CLI_SAFER_REALLOC(handle->files,
CLI_SAFER_REALLOC_OR_GOTO_DONE(handle->files,
sizeof(egg_file*) * (handle->nFiles + 1),
egg_free_egg_file(found_file),
status = CL_EMEM);
Expand All @@ -1689,7 +1689,7 @@ cl_error_t cli_egg_open(fmap_t* map, void** hArchive, char*** comments, uint32_t
} else {
/* Add block to list. */
if (handle->bSolid) {
CLI_SAFER_REALLOC(handle->blocks,
CLI_SAFER_REALLOC_OR_GOTO_DONE(handle->blocks,
sizeof(egg_block*) * (handle->nBlocks + 1),
egg_free_egg_block(found_block),
status = CL_EMEM);
Expand All @@ -1707,7 +1707,7 @@ cl_error_t cli_egg_open(fmap_t* map, void** hArchive, char*** comments, uint32_t
} else {
eggFile = handle->files[handle->nFiles - 1];

CLI_SAFER_REALLOC(eggFile->blocks,
CLI_SAFER_REALLOC_OR_GOTO_DONE(eggFile->blocks,
sizeof(egg_block*) * (eggFile->nBlocks + 1),
egg_free_egg_block(found_block),
status = CL_EMEM);
Expand Down Expand Up @@ -1785,7 +1785,7 @@ cl_error_t cli_egg_open(fmap_t* map, void** hArchive, char*** comments, uint32_t
/*
* Comment found. Add comment to our list.
*/
CLI_SAFER_REALLOC(handle->comments,
CLI_SAFER_REALLOC_OR_GOTO_DONE(handle->comments,
sizeof(char*) * (handle->nComments + 1),
free(comment),
status = CL_EMEM);
Expand Down Expand Up @@ -1972,7 +1972,7 @@ cl_error_t cli_egg_deflate_decompress(char* compressed, size_t compressed_size,
while (zstat == Z_OK && stream.avail_in) {
/* extend output capacity if needed,*/
if (stream.avail_out == 0) {
CLI_SAFER_REALLOC(decoded,
CLI_SAFER_REALLOC_OR_GOTO_DONE(decoded,
capacity + BUFSIZ,
cli_errmsg("cli_egg_deflate_decompress: cannot reallocate memory for decompressed output\n"),
status = CL_EMEM);
Expand Down Expand Up @@ -2094,7 +2094,7 @@ cl_error_t cli_egg_bzip2_decompress(char* compressed, size_t compressed_size, ch
while (bzstat == BZ_OK && stream.avail_in) {
/* extend output capacity if needed,*/
if (stream.avail_out == 0) {
CLI_SAFER_REALLOC(decoded,
CLI_SAFER_REALLOC_OR_GOTO_DONE(decoded,
capacity + BUFSIZ,
cli_errmsg("cli_egg_bzip2_decompress: cannot reallocate memory for decompressed output\n");
status = CL_EMEM);
Expand Down Expand Up @@ -2211,7 +2211,7 @@ cl_error_t cli_egg_lzma_decompress(char* compressed, size_t compressed_size, cha
while (lzmastat == LZMA_RESULT_OK && stream.avail_in) {
/* extend output capacity if needed,*/
if (stream.avail_out == 0) {
CLI_SAFER_REALLOC(decoded,
CLI_SAFER_REALLOC_OR_GOTO_DONE(decoded,
capacity + BUFSIZ,
cli_errmsg("cli_egg_lzma_decompress: cannot reallocate memory for decompressed output\n");
status = CL_EMEM);
Expand Down Expand Up @@ -2361,7 +2361,7 @@ cl_error_t cli_egg_extract_file(void* hArchive, const char** filename, const cha
break;
}

CLI_SAFER_REALLOC(decompressed,
CLI_SAFER_REALLOC_OR_GOTO_DONE(decompressed,
(size_t)decompressed_size + currBlock->blockHeader->compress_size,
cli_errmsg("cli_egg_extract_file: Failed to allocate %" PRIu64 " bytes for decompressed file!\n",
decompressed_size),
Expand All @@ -2386,7 +2386,7 @@ cl_error_t cli_egg_extract_file(void* hArchive, const char** filename, const cha
goto done;
}
/* Decompressed block. Add it to the file data */
CLI_SAFER_REALLOC(decompressed,
CLI_SAFER_REALLOC_OR_GOTO_DONE(decompressed,
(size_t)decompressed_size + decompressed_block_size,
cli_errmsg("cli_egg_extract_file: Failed to allocate %" PRIu64 " bytes for decompressed file!\n",
decompressed_size),
Expand Down Expand Up @@ -2415,7 +2415,7 @@ cl_error_t cli_egg_extract_file(void* hArchive, const char** filename, const cha
goto done;
}
/* Decompressed block. Add it to the file data */
CLI_SAFER_REALLOC(decompressed,
CLI_SAFER_REALLOC_OR_GOTO_DONE(decompressed,
(size_t)decompressed_size + decompressed_block_size,
cli_errmsg("cli_egg_extract_file: Failed to allocate %" PRIu64 " bytes for decompressed file!\n",
decompressed_size),
Expand Down Expand Up @@ -2454,7 +2454,7 @@ cl_error_t cli_egg_extract_file(void* hArchive, const char** filename, const cha
// goto done;
// }
// /* Decompressed block. Add it to the file data */
// CLI_SAFER_REALLOC(decompressed,
// CLI_SAFER_REALLOC_OR_GOTO_DONE(decompressed,
// (size_t)decompressed_size + decompressed_block_size,
// cli_errmsg("cli_egg_extract_file: Failed to allocate %" PRIu64 " bytes for decompressed file!\n",
// decompressed_size),
Expand Down
4 changes: 2 additions & 2 deletions libclamav/entconv.c
Original file line number Diff line number Diff line change
Expand Up @@ -656,7 +656,7 @@ static iconv_t iconv_open_cached(const char* fromcode)
idx = cache->last++;
if (idx >= cache->len) {
cache->len += 16;
cache->tab = cli_max_realloc2(cache->tab, cache->len * sizeof(cache->tab[0]));
cache->tab = cli_max_realloc_or_free(cache->tab, cache->len * sizeof(cache->tab[0]));
if (!cache->tab) {
cli_dbgmsg(MODULE_NAME "!Out of mem in iconv-pool\n");
errno = ENOMEM;
Expand Down Expand Up @@ -1121,7 +1121,7 @@ char* cli_utf16_to_utf8(const char* utf16, size_t length, encoding_t type)
char* s2;

if (length < 2)
return cli_strdup("");
return cli_safer_strdup("");
if (length % 2) {
cli_warnmsg("utf16 length is not multiple of two: %lu\n", (long)length);
length--;
Expand Down
4 changes: 2 additions & 2 deletions libclamav/events.c
Original file line number Diff line number Diff line change
Expand Up @@ -294,7 +294,7 @@ void cli_event_data(cli_events_t *ctx, unsigned id, const void *data, uint32_t l
}
switch (ev->multiple) {
case multiple_last: {
void *v_data = cli_safer_realloc2(ev->u.v_data, len);
void *v_data = cli_safer_realloc_or_free(ev->u.v_data, len);
if (v_data) {
ev->u.v_data = v_data;
memcpy(v_data, data, len);
Expand All @@ -305,7 +305,7 @@ void cli_event_data(cli_events_t *ctx, unsigned id, const void *data, uint32_t l
break;
}
case multiple_concat: {
void *v_data = cli_safer_realloc2(ev->u.v_data, ev->count + len);
void *v_data = cli_safer_realloc_or_free(ev->u.v_data, ev->count + len);
if (v_data) {
ev->u.v_data = v_data;
memcpy((char *)v_data + ev->count, data, len);
Expand Down
8 changes: 4 additions & 4 deletions libclamav/fmap.c
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,7 @@ fmap_t *fmap_check_empty(int fd, off_t offset, size_t len, int *empty, const cha
m->mtime = (uint64_t)st.st_mtime;

if (NULL != name) {
m->name = cli_strdup(name);
m->name = cli_safer_strdup(name);
if (NULL == m->name) {
funmap(m);
return NULL;
Expand Down Expand Up @@ -220,7 +220,7 @@ fmap_t *fmap_check_empty(int fd, off_t offset, size_t len, int *empty, const cha
m->unmap = unmap_win32;

if (NULL != name) {
m->name = cli_strdup(name);
m->name = cli_safer_strdup(name);
if (NULL == m->name) {
funmap(m);
return NULL;
Expand Down Expand Up @@ -293,7 +293,7 @@ fmap_t *fmap_duplicate(cl_fmap_t *map, size_t offset, size_t length, const char
}

if (NULL != name) {
duplicate_map->name = cli_strdup(name);
duplicate_map->name = cli_safer_strdup(name);
if (NULL == duplicate_map->name) {
status = CL_EMEM;
goto done;
Expand Down Expand Up @@ -862,7 +862,7 @@ fmap_t *fmap_open_memory(const void *start, size_t len, const char *name)

if (NULL != name) {
/* Copy the name, if one is given */
m->name = cli_strdup(name);
m->name = cli_safer_strdup(name);
if (NULL == m->name) {
cli_warnmsg("fmap: failed to duplicate map name\n");
goto done;
Expand Down
Loading

0 comments on commit 405829e

Please sign in to comment.