open-dsi
/
clamav
mirror of https://github.com/Cisco-Talos/clamav
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Fixed heap buffer overflow while loading signatures

Browse Source 
There is a possible overflow read when loading PDB and WDB phishing
signatures.

This issue is not a vulnerability.

Changed const char pointers to uint8_t pointers when they are to be used
with data, as well as removing asserts and adding additional error
checking.

Thank you Michał Dardas for reporting this issue.

This fix also resolves:
- https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=43845
- https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=43812
- https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=43866

This commit also fixes a minor leak of pattern matching trans nodes
that was observed when testing with the MPOOL module disabled.
pull/597/head
ragusaa 3 years ago committed by GitHub
parent 3dff2e2f52
commit 1c6746853f
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
6 changed files with 311 additions and 97 deletions
Show all changes Ignore whitespace when comparing lines Ignore changes in amount of whitespace Ignore changes in whitespace at EOL
Show Stats Download Patch File Download Diff File
  1. 62
      libclamav/matcher-ac.c
  2. 38
      libclamav/others.h
  3. 266
      libclamav/regex_list.c
  4. 36
      libclamav/regex_suffix.c
  5. 4
      libclamav/unzip.c
  6. 2
      unit_tests/check_regex.c

62
libclamav/matcher-ac.c
Unescape View File

@ -641,10 +641,36 @@ static void ac_free_special(struct cli_ac_patt *p)
MPOOL_FREE(mempool, p->special_table);
}
/*
* This is a test to see if we have already seen this pointer. If we have, we
* have already freed it, so don't do it again (double-free)
*/
static int need_to_free_trans(struct cli_matcher *root, const size_t idx)
{
size_t j;
size_t min = idx;
if (root->ac_nodes < idx) {
/*Should never happen, but check just to be safe.*/
min = root->ac_nodes;
}
for (j = 0; j < min; j++) {
if (NULL == root->ac_nodetable[j]) {
continue;
}
if (root->ac_nodetable[idx]->trans == root->ac_nodetable[j]->trans) {
return 0;
}
}
return 1;
}
void cli_ac_free(struct cli_matcher *root)
{
uint32_t i;
struct cli_ac_patt *patt;
uint32_t i = 0;
struct cli_ac_patt *patt = NULL;
for (i = 0; i < root->ac_patterns; i++) {
patt = root->ac_pattable[i];
@ -655,45 +681,55 @@ void cli_ac_free(struct cli_matcher *root)
TODO: never store the virname in the ac pattern and only store it per-signature, not per-pattern. */
MPOOL_FREE(root->mempool, patt->virname);
}
if (patt->special)
if (patt->special) {
mpool_ac_free_special(root->mempool, patt);
}
MPOOL_FREE(root->mempool, patt);
}
if (root->ac_pattable)
if (root->ac_pattable) {
MPOOL_FREE(root->mempool, root->ac_pattable);
}
if (root->ac_reloff)
if (root->ac_reloff) {
MPOOL_FREE(root->mempool, root->ac_reloff);
}
/* Freeing trans nodes must be done before freeing table nodes! */
for (i = 0; i < root->ac_nodes; i++) {
if (!IS_LEAF(root->ac_nodetable[i]) &&
root->ac_nodetable[i]->fail &&
root->ac_nodetable[i]->trans != root->ac_nodetable[i]->fail->trans) {
MPOOL_FREE(root->mempool, root->ac_nodetable[i]->trans);
root->ac_root->trans != root->ac_nodetable[i]->trans) {
if (need_to_free_trans(root, i)) {
MPOOL_FREE(root->mempool, root->ac_nodetable[i]->trans);
}
}
}
for (i = 0; i < root->ac_lists; i++)
for (i = 0; i < root->ac_lists; i++) {
MPOOL_FREE(root->mempool, root->ac_listtable[i]);
}
if (root->ac_listtable)
if (root->ac_listtable) {
MPOOL_FREE(root->mempool, root->ac_listtable);
}
for (i = 0; i < root->ac_nodes; i++)
for (i = 0; i < root->ac_nodes; i++) {
MPOOL_FREE(root->mempool, root->ac_nodetable[i]);
}
if (root->ac_nodetable)
if (root->ac_nodetable) {
MPOOL_FREE(root->mempool, root->ac_nodetable);
}
if (root->ac_root) {
MPOOL_FREE(root->mempool, root->ac_root->trans);
MPOOL_FREE(root->mempool, root->ac_root);
}
if (root->filter)
if (root->filter) {
MPOOL_FREE(root->mempool, root->filter);
}
}
/*

38
libclamav/others.h
Unescape View File

@ -1227,6 +1227,19 @@ uint8_t cli_set_debug_flag(uint8_t debug_flag);
} while (0)
#endif
#ifndef CLI_STRDUP
#define CLI_STRDUP(buf, var, ...) \
do { \
var = cli_strdup(buf); \
if (NULL == var) { \
do { \
__VA_ARGS__; \
} while (0); \
goto done; \
} \
} while (0)
#endif
#ifndef FREE
#define FREE(var) \
do { \
@ -1326,4 +1339,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 ptr if size==0. It is safe to free ptr after `done:`.
*
* @param ptr
* @param size
* @return void*
*/
#ifndef CLI_REALLOC
#define CLI_REALLOC(ptr, size, ...) \
do { \
void *vTmp = cli_realloc(ptr, size); \
if (NULL == vTmp) { \
do { \
__VA_ARGS__; \
} while (0); \
goto done; \
} \
ptr = vTmp; \
} while (0)
#endif
#endif

266
libclamav/regex_list.c
Unescape View File

@ -39,7 +39,6 @@
#include <limits.h>
#include <sys/types.h>
#include <assert.h>
#include "regex/regex.h"
@ -162,13 +161,35 @@ cl_error_t regex_list_match(struct regex_matcher *matcher, char *real_url, const
struct cli_ac_data mdata;
struct cli_ac_result *res = NULL;
assert(matcher);
assert(real_url);
assert(display_url);
if (NULL == matcher) {
rc = CL_ENULLARG;
cli_errmsg("regex_list_match: matcher must be initialized\n");
goto done;
}
if (NULL == real_url) {
rc = CL_ENULLARG;
cli_errmsg("regex_list_match: real_url must be initialized\n");
goto done;
}
if (NULL == display_url) {
rc = CL_ENULLARG;
cli_errmsg("regex_list_match: display_url must be initialized\n");
goto done;
}
*info = NULL;
if (!matcher->list_inited)
return CL_SUCCESS;
assert(matcher->list_built);
if (1 != matcher->list_inited) {
rc = CL_SUCCESS;
goto done;
}
if (0 == matcher->list_built) {
cli_errmsg("regex_list_match: matcher->list_built must be initialized\n");
rc = CL_ENULLARG;
goto done;
}
/* skip initial '.' inserted by get_host */
if (real_url[0] == '.') real_url++;
if (display_url[0] == '.') display_url++;
@ -279,6 +300,7 @@ cl_error_t regex_list_match(struct regex_matcher *matcher, char *real_url, const
cli_dbgmsg("Lookup result: not in regex list\n");
else
cli_dbgmsg("Lookup result: in regex list\n");
done:
return rc;
}
@ -287,11 +309,25 @@ cl_error_t regex_list_match(struct regex_matcher *matcher, char *real_url, const
cl_error_t init_regex_list(struct regex_matcher *matcher, uint8_t dconf_prefiltering)
{
#ifdef USE_MPOOL
mpool_t *mp = matcher->mempool;
mpool_t *mp = NULL;
#endif
cl_error_t rc = CL_SUCCESS;
if (NULL == matcher) {
cli_errmsg("init_regex_list: matcher must be initialized\n");
rc = CL_ENULLARG;
goto done;
}
#ifdef USE_MPOOL
mp = matcher->mempool;
if (NULL == mp) {
cli_errmsg("init_regex_list: matcher->mempool must be initialized\n");
rc = CL_ENULLARG;
goto done;
}
#endif
cl_error_t rc;
assert(matcher);
memset(matcher, 0, sizeof(*matcher));
matcher->list_inited = 1;
@ -301,23 +337,25 @@ cl_error_t init_regex_list(struct regex_matcher *matcher, uint8_t dconf_prefilte
#ifdef USE_MPOOL
matcher->mempool = mp;
matcher->suffixes.mempool = mp;
assert(mp && "mempool must be initialized");
#endif
if ((rc = cli_ac_init(&matcher->suffixes, 2, 32, dconf_prefiltering))) {
return rc;
goto done;
}
#ifdef USE_MPOOL
matcher->sha256_hashes.mempool = mp;
matcher->hostkey_prefix.mempool = mp;
#endif
if ((rc = cli_bm_init(&matcher->sha256_hashes))) {
return rc;
goto done;
}
if ((rc = cli_bm_init(&matcher->hostkey_prefix))) {
return rc;
goto done;
}
filter_init(&matcher->filter);
return CL_SUCCESS;
done:
return rc;
}
static int functionality_level_check(char *line)
@ -424,7 +462,10 @@ cl_error_t load_regex_matcher(struct cl_engine *engine, struct regex_matcher *ma
int line = 0, entry = 0;
char buffer[FILEBUFF];
assert(matcher);
if (NULL == matcher) {
cli_errmsg("load_regex_matcher: matcher must be initialized\n");
return CL_ENULLARG;
}
if (matcher->list_inited == -1)
return CL_EMALFDB; /* already failed to load */
@ -510,8 +551,9 @@ cl_error_t load_regex_matcher(struct cl_engine *engine, struct regex_matcher *ma
if ((buffer[0] == 'R' && !is_allow_list_lookup) || ((buffer[0] == 'X' || buffer[0] == 'Y') && is_allow_list_lookup)) {
/* regex for hostname*/
if ((rc = regex_list_add_pattern(matcher, pattern)))
if ((rc = regex_list_add_pattern(matcher, pattern))) {
return rc == CL_EMEM ? CL_EMEM : CL_EMALFDB;
}
} else if ((buffer[0] == 'H' && !is_allow_list_lookup) || (buffer[0] == 'M' && is_allow_list_lookup)) {
/*matches displayed host*/
if ((rc = add_static_pattern(matcher, pattern)))
@ -564,7 +606,10 @@ cl_error_t cli_build_regex_list(struct regex_matcher *matcher)
/* Done with this matcher, free resources */
void regex_list_done(struct regex_matcher *matcher)
{
assert(matcher);
if (NULL == matcher) {
cli_errmsg("regex_list_done: matcher must be initialized\n");
goto done;
}
if (matcher->list_inited == 1) {
size_t i;
@ -594,24 +639,55 @@ void regex_list_done(struct regex_matcher *matcher)
cli_bm_free(&matcher->sha256_hashes);
cli_bm_free(&matcher->hostkey_prefix);
}
done:
return;
}
int is_regex_ok(struct regex_matcher *matcher)
{
assert(matcher);
return (!matcher->list_inited || matcher->list_inited != -1); /* either we don't have a regexlist, or we initialized it successfully */
int ret = 0;
if (NULL == matcher) {
cli_errmsg("is_regex_ok: matcher must be initialized\n");
} else {
ret = (!matcher->list_inited || matcher->list_inited != -1); /* either we don't have a regexlist, or we initialized it successfully */
}
return ret;
}
static int add_newsuffix(struct regex_matcher *matcher, struct regex_list *info, const char *suffix, size_t len)
static cl_error_t add_newsuffix(struct regex_matcher *matcher, struct regex_list *info, const char *suffix, size_t len)
{
struct cli_matcher *root = &matcher->suffixes;
struct cli_ac_patt *new = MPOOL_CALLOC(matcher->mempool, 1, sizeof(*new));
struct cli_matcher *root = NULL;
struct cli_ac_patt *new = NULL;
size_t i;
int ret;
cl_error_t ret = CL_SUCCESS;
if (!new)
return CL_EMEM;
assert(root && suffix);
if (NULL == matcher) {
cli_errmsg("add_newsuffix: matcher must be initialized\n");
ret = CL_ENULLARG;
goto done;
}
root = &matcher->suffixes;
if (NULL == root) {
cli_errmsg("add_newsuffix: root must be initialized\n");
ret = CL_ENULLARG;
goto done;
}
if (NULL == suffix) {
cli_errmsg("add_newsuffix: suffix must be initialized\n");
ret = CL_ENULLARG;
goto done;
}
new = MPOOL_CALLOC(matcher->mempool, 1, sizeof(*new));
if (!new) {
cli_errmsg("add_newsuffix: Unable to allocate memory for new\n");
ret = CL_EMEM;
goto done;
}
new->rtype = 0;
new->type = 0;
@ -629,22 +705,38 @@ static int add_newsuffix(struct regex_matcher *matcher, struct regex_list *info,
new->pattern = MPOOL_MALLOC(matcher->mempool, sizeof(new->pattern[0]) * len);
if (!new->pattern) {
MPOOL_FREE(matcher->mempool, new);
cli_errmsg("add_newsuffix: Unable to allocate memory for new->pattern\n");
return CL_EMEM;
ret = CL_EMEM;
goto done;
}
for (i = 0; i < len; i++)
for (i = 0; i < len; i++) {
new->pattern[i] = suffix[i]; /*new->pattern is short int* */
}
new->customdata = info;
new->virname = NULL;
if ((ret = cli_ac_addpatt(root, new))) {
MPOOL_FREE(matcher->mempool, new->pattern);
MPOOL_FREE(matcher->mempool, new);
return ret;
goto done;
}
filter_add_static(&matcher->filter, (const unsigned char *)suffix, len, "regex");
return CL_SUCCESS;
if (filter_add_static(&matcher->filter, (const unsigned char *)suffix, len, "regex") < 0) {
cli_errmsg("add_newsuffix: Unable to add filter\n");
ret = CL_ERROR;
goto done;
}
done:
if (CL_SUCCESS != ret) {
if (NULL != new) {
if (NULL != new->pattern) {
MPOOL_FREE(matcher->mempool, new->pattern);
}
MPOOL_FREE(matcher->mempool, new);
}
}
return ret;
}
#define MODULE "regex_list: "
@ -663,45 +755,84 @@ static void list_add_tail(struct regex_list_ht *ht, struct regex_list *regex)
static cl_error_t add_pattern_suffix(void *cbdata, const char *suffix, size_t suffix_len, const struct regex_list *iregex)
{
struct regex_matcher *matcher = cbdata;
struct regex_list *regex = cli_malloc(sizeof(*regex));
const struct cli_element *el;
void *tmp_matcher; /* save original address if OOM occurs */
struct regex_list *regex = NULL;
const struct cli_element *el = NULL;
void *tmp_matcher = NULL; /* save original address if OOM occurs */
cl_error_t ret = CL_SUCCESS;
assert(matcher);
if (!regex) {
cli_errmsg("add_pattern_suffix: Unable to allocate memory for regex\n");
return CL_EMEM;
if (NULL == matcher) {
cli_errmsg("add_pattern_suffix: matcher must be initialized\n");
ret = CL_ENULLARG;
goto done;
}
if (NULL == suffix) {
cli_errmsg("add_pattern_suffix: suffix must be initialized\n");
ret = CL_ENULLARG;
goto done;
}
regex->pattern = iregex->pattern ? cli_strdup(iregex->pattern) : NULL;
regex->preg = iregex->preg;
regex->nxt = NULL;
el = cli_hashtab_find(&matcher->suffix_hash, suffix, suffix_len);
if (NULL == iregex) {
cli_errmsg("add_pattern_suffix: iregex must be initialized\n");
ret = CL_ENULLARG;
goto done;
}
CLI_MALLOC(regex, sizeof(*regex),
cli_errmsg("add_pattern_suffix: Unable to allocate memory for regex\n");
ret = CL_EMEM);
if (NULL == iregex->pattern) {
regex->pattern = NULL;
} else {
CLI_STRDUP(iregex->pattern, regex->pattern,
cli_errmsg("add_pattern_suffix: unable to strdup iregex->pattern");
ret = CL_EMEM);
}
regex->preg = iregex->preg;
regex->nxt = NULL;
el = cli_hashtab_find(&matcher->suffix_hash, suffix, suffix_len);
/* TODO: what if suffixes are prefixes of eachother and only one will
* match? */
if (el) {
/* existing suffix */
assert((size_t)el->data < matcher->suffix_cnt);
if ((size_t)el->data >= matcher->suffix_cnt) {
cli_errmsg("add_pattern_suffix: el-> data too large");
ret = CL_ERROR;
goto done;
}
list_add_tail(&matcher->suffix_regexes[(size_t)el->data], regex);
} else {
/* new suffix */
size_t n = matcher->suffix_cnt;
el = cli_hashtab_insert(&matcher->suffix_hash, suffix, suffix_len, (cli_element_data)n);
tmp_matcher = matcher->suffix_regexes; /* save the current value before cli_realloc() */
tmp_matcher = cli_realloc(matcher->suffix_regexes, (n + 1) * sizeof(*matcher->suffix_regexes));
if (!tmp_matcher) {
FREE(regex->pattern);
free(regex);
return CL_EMEM;
}
matcher->suffix_regexes = tmp_matcher; /* success, point at new memory location */
CLI_REALLOC(matcher->suffix_regexes,
(n + 1) * sizeof(*matcher->suffix_regexes),
cli_errmsg("add_pattern_suffix: Unable to reallocate memory for matcher->suffix_regexes\n");
ret = CL_EMEM);
matcher->suffix_regexes[n].tail = regex;
matcher->suffix_regexes[n].head = regex;
matcher->suffix_cnt++;
if (suffix[0] == '/' && suffix[1] == '\0')
if (suffix[0] == '/' && suffix[1] == '\0') {
matcher->root_regex_idx = n;
add_newsuffix(matcher, regex, suffix, suffix_len);
}
ret = add_newsuffix(matcher, regex, suffix, suffix_len);
if (CL_SUCCESS != ret) {
cli_hashtab_delete(&matcher->suffix_hash, suffix, suffix_len);
/*shrink the size back to what it was.*/
CLI_REALLOC(matcher->suffix_regexes, n * sizeof(*matcher->suffix_regexes));
} else {
matcher->suffix_cnt++;
}
}
return CL_SUCCESS;
done:
if (CL_SUCCESS != ret) {
FREE(regex->pattern);
FREE(regex);
}
return ret;
}
static size_t reverse_string(char *pattern)
@ -737,14 +868,17 @@ static cl_error_t add_static_pattern(struct regex_matcher *matcher, char *patter
{
size_t len;
struct regex_list regex;
cl_error_t rc;
len = reverse_string(pattern);
regex.nxt = NULL;
regex.pattern = cli_strdup(pattern);
regex.preg = NULL;
rc = add_pattern_suffix(matcher, pattern, len, &regex);
free(regex.pattern);
cl_error_t rc = CL_EMEM;
len = reverse_string(pattern);
regex.nxt = NULL;
CLI_STRDUP(pattern, regex.pattern,
cli_errmsg("add_static_pattern: Cannot allocate memory for regex.pattern\n");
rc = CL_EMEM);
regex.preg = NULL;
rc = add_pattern_suffix(matcher, pattern, len, &regex);
done:
FREE(regex.pattern);
return rc;
}

36
libclamav/regex_suffix.c
Unescape View File

@ -27,7 +27,6 @@
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <assert.h>
#include "clamav.h"
#include "others.h"
@ -180,7 +179,7 @@ static void destroy_tree(struct node *n)
free(n);
}
static uint8_t *parse_char_class(const char *pat, size_t *pos)
static uint8_t *parse_char_class(const uint8_t *pat, size_t *pos)
{
unsigned char range_start = 0;
int hasprev = 0;
@ -200,7 +199,10 @@ static uint8_t *parse_char_class(const char *pat, size_t *pos)
/* it is a range*/
unsigned char range_end;
unsigned int c;
assert(range_start);
if (0 == range_start) {
cli_errmsg("parse_char_class: range_start not initialized");
return NULL;
}
++*pos;
if (pat[*pos] == '[')
if (pat[*pos + 1] == '.') {
@ -238,7 +240,7 @@ static uint8_t *parse_char_class(const char *pat, size_t *pos)
return bitmap;
}
static struct node *parse_regex(const char *p, size_t *last)
static struct node *parse_regex(const uint8_t *p, size_t *last)
{
struct node *v = NULL;
struct node *right;
@ -426,14 +428,15 @@ static cl_error_t build_suffixtree_descend(struct node *n, struct text_buffer *b
cl_error_t cli_regex2suffix(const char *pattern, regex_t *preg, suffix_callback cb, void *cbdata)
{
struct regex_list regex;
struct text_buffer buf;
struct node root_node;
struct node *n;
size_t last = 0;
struct regex_list regex = {0};
struct text_buffer buf = {0};
struct node root_node = {0};
struct node *n = NULL;
size_t last = 0;
int rc;
assert(pattern);
VERIFY_POINTER(pattern, cli_errmsg("cli_regex2suffix: pattern must be initialized"); rc = CL_ENULLARG);
VERIFY_POINTER(preg, cli_errmsg("cli_regex2suffix: preg must be initialized"); rc = CL_ENULLARG);
regex.preg = preg;
rc = cli_regcomp(regex.preg, pattern, REG_EXTENDED);
@ -449,10 +452,10 @@ cl_error_t cli_regex2suffix(const char *pattern, regex_t *preg, suffix_callback
}
return rc;
}
regex.nxt = NULL;
regex.pattern = cli_strdup(pattern);
regex.nxt = NULL;
CLI_STRDUP(pattern, regex.pattern, cli_errmsg("cli_regex2suffix: Unable to duplicate pattern"); rc = CL_EMEM);
n = parse_regex(pattern, &last);
n = parse_regex(((const uint8_t *)pattern), &last);
if (!n)
return REG_ESPACE;
memset(&buf, 0, sizeof(buf));
@ -460,8 +463,11 @@ cl_error_t cli_regex2suffix(const char *pattern, regex_t *preg, suffix_callback
n->parent = &root_node;
rc = build_suffixtree_descend(n, &buf, cb, cbdata, &regex);
free(regex.pattern);
free(buf.data);
done:
FREE(regex.pattern);
FREE(buf.data);
destroy_tree(n);
return rc;
}

4
libclamav/unzip.c
Unescape View File

@ -113,8 +113,8 @@ static cl_error_t unz(
zip_cb zcb,
const char *original_filename)
{
char obuf[BUFSIZ];
char *tempfile = NULL;
char obuf[BUFSIZ] = {0};
char *tempfile = NULL;
int out_file, ret = CL_CLEAN;
int res = 1;
size_t written = 0;

2
unit_tests/check_regex.c
Unescape View File

@ -219,7 +219,7 @@ static const struct rtest {
{NULL,
"http://key.com", "go to key.com", RTR_CLEAN},
{":.+\\.paypal\\.(com|de|fr|it)([/?].*)?:.+\\.ebay\\.(at|be|ca|ch|co\\.uk|de|es|fr|ie|in|it|nl|ph|pl|com(\\.(au|cn|hk|my|sg))?)([/?].*)?/",
"http://www.paypal.com", "pics.ebay.com", RTR_ALLOWED},
"http://www.paypal.com", "pics.ebay.com", RTR_INVALID_REGEX},
{NULL, "http://somefakeurl.example.com", "someotherdomain-key.com", RTR_CLEAN},
{NULL, "http://somefakeurl.example.com", "someotherdomain.key.com", RTR_PHISH},
{NULL, "http://malware-test.example.com/something", "test", RTR_BLOCKED},

Write Preview
Loading…
Cancel
Save