Skip to content

Commit 0fa734a

Browse files
committed
Fixed heap buffer overflow while loading signatures
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.
1 parent 2c91aa7 commit 0fa734a

5 files changed

Lines changed: 292 additions & 82 deletions

File tree

libclamav/matcher-ac.c

Lines changed: 39 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -641,10 +641,34 @@ static void ac_free_special(struct cli_ac_patt *p)
641641
MPOOL_FREE(mempool, p->special_table);
642642
}
643643

644+
/*Find all duplicate pointers and zero them so that we won't get double-free
645+
* errors when we iterate over the cli_matcher to free all the trans
646+
* pointers.*/
647+
static void zero_duplicate_trans_nodes(struct cli_matcher *root, const size_t idx)
648+
{
649+
size_t j;
650+
for (j = 0; j < root->ac_nodes; j++) {
651+
if (j == idx) {
652+
continue;
653+
}
654+
if (NULL == root->ac_nodetable[j]) {
655+
continue;
656+
}
657+
if (root->ac_nodetable[idx]->trans == root->ac_nodetable[j]->trans) {
658+
root->ac_nodetable[j]->trans = NULL;
659+
}
660+
}
661+
if (root->ac_root) {
662+
if (root->ac_nodetable[idx]->trans == root->ac_root->trans) {
663+
root->ac_root->trans = NULL;
664+
}
665+
}
666+
}
667+
644668
void cli_ac_free(struct cli_matcher *root)
645669
{
646-
uint32_t i;
647-
struct cli_ac_patt *patt;
670+
uint32_t i = 0;
671+
struct cli_ac_patt *patt = NULL;
648672

649673
for (i = 0; i < root->ac_patterns; i++) {
650674
patt = root->ac_pattable[i];
@@ -671,7 +695,9 @@ void cli_ac_free(struct cli_matcher *root)
671695
if (!IS_LEAF(root->ac_nodetable[i]) &&
672696
root->ac_nodetable[i]->fail &&
673697
root->ac_nodetable[i]->trans != root->ac_nodetable[i]->fail->trans) {
698+
zero_duplicate_trans_nodes(root, i);
674699
MPOOL_FREE(root->mempool, root->ac_nodetable[i]->trans);
700+
root->ac_nodetable[i]->trans = NULL;
675701
}
676702
}
677703

@@ -681,8 +707,18 @@ void cli_ac_free(struct cli_matcher *root)
681707
if (root->ac_listtable)
682708
MPOOL_FREE(root->mempool, root->ac_listtable);
683709

684-
for (i = 0; i < root->ac_nodes; i++)
710+
for (i = 0; i < root->ac_nodes; i++) {
711+
if (NULL == root->ac_nodetable[i]) {
712+
continue;
713+
}
714+
if (!IS_LEAF(root->ac_nodetable[i])) {
715+
zero_duplicate_trans_nodes(root, i);
716+
MPOOL_FREE(root->mempool, root->ac_nodetable[i]->trans);
717+
root->ac_nodetable[i]->trans = NULL;
718+
}
685719
MPOOL_FREE(root->mempool, root->ac_nodetable[i]);
720+
root->ac_nodetable[i] = NULL;
721+
}
686722

687723
if (root->ac_nodetable)
688724
MPOOL_FREE(root->mempool, root->ac_nodetable);

libclamav/others.h

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1177,6 +1177,19 @@ uint8_t cli_set_debug_flag(uint8_t debug_flag);
11771177
} while (0)
11781178
#endif
11791179

1180+
#ifndef CLI_STRDUP
1181+
#define CLI_STRDUP(buf, var, ...) \
1182+
do { \
1183+
var = cli_strdup(buf); \
1184+
if (NULL == var) { \
1185+
do { \
1186+
__VA_ARGS__; \
1187+
} while (0); \
1188+
goto done; \
1189+
} \
1190+
} while (0)
1191+
#endif
1192+
11801193
#ifndef FREE
11811194
#define FREE(var) \
11821195
do { \
@@ -1251,4 +1264,29 @@ uint8_t cli_set_debug_flag(uint8_t debug_flag);
12511264
} while (0)
12521265
#endif
12531266

1267+
/**
1268+
* @brief Wrapper around realloc that limits how much may be allocated to CLI_MAX_ALLOCATION.
1269+
*
1270+
* IMPORTANT: This differs from realloc() in that if size==0, it will NOT free the ptr.
1271+
*
1272+
* NOTE: cli_realloc() will NOT free ptr if size==0. It is safe to free ptr after `done:`.
1273+
*
1274+
* @param ptr
1275+
* @param size
1276+
* @return void*
1277+
*/
1278+
#ifndef CLI_REALLOC
1279+
#define CLI_REALLOC(ptr, size, ...) \
1280+
do { \
1281+
void *vTmp = cli_realloc(ptr, size); \
1282+
if (NULL == vTmp) { \
1283+
do { \
1284+
__VA_ARGS__; \
1285+
} while (0); \
1286+
goto done; \
1287+
} \
1288+
ptr = vTmp; \
1289+
} while (0)
1290+
#endif
1291+
12541292
#endif

0 commit comments

Comments
 (0)