More Coverity bug fixes

Fixed the following Coverity issues:
- 225236 - In cli_egg_extract_file: Dereference of an explicit
null value (CWE-476). The first fail case checked handle for
NULL and then dereferenced it in the done block

- 225209 - In executeIfNewVersion: Leak of memory or pointers
to system resources (CWE-404). modifiedCommand was defined
twice, with the inner instance being assigned to and the
outer instance being freed

- 225201 - In regex_list_match: Code can never be reached
because of a logical contradiction (CWE-561). The code had
logic off to the side that may have been missed:

filter_search_rc = filter_search(&matcher->filter, (const unsigned char *)bufrev, buffer_len) != -1;
if (filter_search_rc == -1) {

- 225198 - In phishingCheck: Leak of memory or pointers to
system resources (CWE-404). A fail case caused by malloc
failing would leak previously allocated memory.

- 225197 - In updatecustomdb: A pointer to freed memory
is dereferenced, used as a function argument, or otherwise
used (CWE-416). In a fail case, a pointer was freed and
then used in a debug print statement

- 225190 - In updatedb: A pointer to freed memory is
dereferenced, used as a function argument, or otherwise used
(CWE-416). In a fail case, a pointer was freed and then used
in a debug print statement

- 225195 - In cli_egg_open: The sizeof operator is used on a
wrong argument that incidentally has the same size (CWE-467).
sizeof(char **) was being used instead of sizeof(char *)

- 225193 - In egg_parse_comment_header: Code can never be
reached because of a logical contradiction (CWE-561).
A cleanup case for variable comment was unnecessary, and
to fix comment was removed entirely.

- 225147 - In get_server_node: Code can never be reached
because of a logical contradiction (CWE-561). A cleanup
case for variable url was unnecessary

- 225168 - In download_complete_callback: Missing break
statement between cases in switch statement (CWE-484).
In the case where forking failed, freshclam would check
the database without forking but then continue on to
execute the code intended to be done in the child process
because of a missing break statement

- 225152 - In cli_egg_lzma_decompress: Use of an
uninitialized variable (CWE-457). Certain fail cases
would call cli_LzmaShutdown on an uninitialized stream.
Now it’s only called after initialization occurs.
pull/130/head
Andrew 5 years ago
parent 1d66184a7d
commit 2429b8dfa7
  1. 10
      freshclam/freshclam.c
  2. 27
      libclamav/egg.c
  3. 2
      libclamav/regex_list.c
  4. 6
      libfreshclam/libfreshclam_internal.c

@ -280,6 +280,7 @@ fc_error_t download_complete_callback(const char *dbFilename, void *context)
status = FC_ETESTFAIL;
goto done;
}
break;
}
case 0: {
/*
@ -450,13 +451,6 @@ static fc_error_t get_server_node(
status = FC_SUCCESS;
done:
if (FC_SUCCESS != status) {
if (NULL != url) {
free(url);
}
}
return status;
}
@ -1363,7 +1357,7 @@ static fc_error_t executeIfNewVersion(
}
version++;
}
char *modifiedCommand = (char *)malloc(strlen(command) + strlen(version) + 10);
modifiedCommand = (char *)malloc(strlen(command) + strlen(version) + 10);
if (NULL == modifiedCommand) {
logg("!executeIfNewVersion: Can't allocate memory for modifiedCommand\n");
status = FC_EMEM;

@ -594,7 +594,6 @@ static cl_error_t egg_parse_comment_header(const uint8_t* index, size_t size, ex
{
cl_error_t status = CL_EPARSE;
char* comment = NULL;
char* comment_utf8 = NULL;
size_t comment_utf8_size = 0;
@ -635,20 +634,12 @@ static cl_error_t egg_parse_comment_header(const uint8_t* index, size_t size, ex
goto done;
}
}
comment = comment_utf8;
cli_dbgmsg("egg_parse_comment_header: comment: %s\n", comment_utf8);
cli_dbgmsg("egg_parse_comment_header: comment: %s\n", comment);
*commentInfo = comment;
*commentInfo = comment_utf8;
status = CL_SUCCESS;
done:
if (CL_SUCCESS != status) {
if (comment) {
free(comment);
}
}
return status;
}
@ -1230,7 +1221,7 @@ static cl_error_t egg_parse_file_extra_field(egg_handle* handle, egg_file* eggFi
comments_tmp = (char**)cli_realloc(
(void*)eggFile->comments,
sizeof(char**) * (eggFile->nComments + 1));
sizeof(char*) * (eggFile->nComments + 1));
if (NULL == comments_tmp) {
free(comment);
status = CL_EMEM;
@ -1808,7 +1799,7 @@ cl_error_t cli_egg_open(fmap_t* map, size_t sfx_offset, void** hArchive, char***
comments_tmp = (char**)cli_realloc(
(void*)handle->comments,
sizeof(char**) * (handle->nComments + 1));
sizeof(char*) * (handle->nComments + 1));
if (NULL == comments_tmp) {
free(comment);
status = CL_EMEM;
@ -2188,6 +2179,7 @@ cl_error_t cli_egg_lzma_decompress(char* compressed, size_t compressed_size, cha
uint32_t declen = 0, capacity = 0;
struct CLI_LZMA stream;
int stream_initialized = 0;
int lzmastat;
if (NULL == compressed || compressed_size == 0 || NULL == decompressed || NULL == decompressed_size) {
@ -2219,6 +2211,7 @@ cl_error_t cli_egg_lzma_decompress(char* compressed, size_t compressed_size, cha
status = CL_EMEM;
goto done;
}
stream_initialized = 1;
/* initial inflate */
lzmastat = cli_LzmaDecode(&stream);
@ -2284,7 +2277,9 @@ cl_error_t cli_egg_lzma_decompress(char* compressed, size_t compressed_size, cha
done:
(void)cli_LzmaShutdown(&stream);
if (stream_initialized) {
(void)cli_LzmaShutdown(&stream);
}
if (CL_SUCCESS != status) {
free(decoded);
@ -2529,7 +2524,9 @@ cl_error_t cli_egg_extract_file(void* hArchive, const char** filename, const cha
status = CL_SUCCESS;
done:
handle->fileExtractionIndex += 1;
if (NULL != handle) {
handle->fileExtractionIndex += 1;
}
if (CL_SUCCESS != status) {
/* Free buffer */

@ -208,7 +208,7 @@ cl_error_t regex_list_match(struct regex_matcher *matcher, char *real_url, const
return CL_EMEM;
reverse_string(bufrev);
filter_search_rc = filter_search(&matcher->filter, (const unsigned char *)bufrev, buffer_len) != -1;
filter_search_rc = filter_search(&matcher->filter, (const unsigned char *)bufrev, buffer_len);
if (filter_search_rc == -1) {
free(buffer);
free(bufrev);

@ -2020,9 +2020,8 @@ fc_error_t updatedb(
}
snprintf(tmpfile_with_extension, tmpfile_with_extension_len + 1, "%s-%s", tmpfile, newLocalFilename);
if (rename(tmpfile, tmpfile_with_extension) == -1) {
free(tmpfile_with_extension);
logg("!updatedb: Can't rename %s to %s: %s\n", tmpfile, tmpfile_with_extension, strerror(errno));
free(tmpfile_with_extension);
status = FC_EDBDIRACCESS;
goto done;
}
@ -2229,9 +2228,8 @@ fc_error_t updatecustomdb(
}
snprintf(tmpfile_with_extension, tmpfile_with_extension_len + 1, "%s-%s", tmpfile, databaseName);
if (rename(tmpfile, tmpfile_with_extension) == -1) {
free(tmpfile_with_extension);
logg("!updatecustomdb: Can't rename %s to %s: %s\n", tmpfile, tmpfile_with_extension, strerror(errno));
free(tmpfile_with_extension);
status = FC_EDBDIRACCESS;
goto done;
}

Loading…
Cancel
Save