Skip to content

Commit

Permalink
Adds detection and heuristic alert for zips with overlapping files, p…
Browse files Browse the repository at this point in the history 
…reventing extraction of non-recursive zip bombs.
  • Loading branch information
@micahsnyder
micahsnyder committed Jul 30, 2019
1 parent 7f92726 commit dcd26ea
Show file tree
Hide file tree
Showing 2 changed files with 72 additions and 17 deletions.
15 changes: 9 additions & 6 deletions NEWS.md
View file
Expand Up @@ -5,14 +5,17 @@ Note: This file refers to the source tarball. Things described here may differ

## 0.101.3

ClamAV 0.101.3 is a patch release...
ClamAV 0.101.3 is a patch release to address a vulnerability to non-recursive
zip bombs.

- Fixes for the following vulnerabilities affecting 0.101.2 and prior:
-
A Denial-of-Service (DoS) vulnerability may occur when scanning a zip bomb as a
result of excessively long scan times. The issue is resolved by detecting the
overlapping local file headers which characterize the non-recursive zip bomb
described by David Fifield,
[here](https://www.bamsoftware.com/hacks/zipbomb/).

Additional thanks to the following community members for submitting bug reports:

-
Thank you to Hanno Böck for reporting the issue as it relates to ClamAV,
[here](https://bugzilla.clamav.net/show_bug.cgi?id=12356).

## 0.101.2

Expand Down
74 changes: 63 additions & 11 deletions libclamav/unzip.c
View file
Expand Up @@ -54,6 +54,8 @@
#define UNZIP_PRIVATE
#include "unzip.h"

#define ZIP_MAX_NUM_OVERLAPPING_FILES 5

#define ZIP_CRC32(r,c,b,l) \
do { \
r = crc32(~c,b,l); \
Expand Down Expand Up @@ -493,14 +495,14 @@ static inline int zdecrypt(const uint8_t *src, uint32_t csize, uint32_t usize, c
if (pass_zip)
pass_zip = pass_zip->next;
else
pass_any = pass_any->next;
pass_any = pass_any->next;
}

cli_dbgmsg("cli_unzip: decrypt - skipping encrypted file, no valid passwords\n");
return CL_SUCCESS;
}

static unsigned int lhdr(fmap_t *map, uint32_t loff,uint32_t zsize, unsigned int *fu, unsigned int fc, const uint8_t *ch, int *ret, cli_ctx *ctx, char *tmpd, int detect_encrypted, zip_cb zcb) {
static unsigned int lhdr(fmap_t *map, uint32_t loff,uint32_t zsize, unsigned int *fu, unsigned int fc, const uint8_t *ch, int *ret, cli_ctx *ctx, char *tmpd, int detect_encrypted, zip_cb zcb, uint32_t *file_local_header_size, uint32_t* file_local_data_size) {
const uint8_t *lh, *zip;
char name[256];
uint32_t csize, usize;
Expand Down Expand Up @@ -563,7 +565,7 @@ static unsigned int lhdr(fmap_t *map, uint32_t loff,uint32_t zsize, unsigned int
}
virus_found = 1;
}

if(LH_flags & F_USEDD) {
cli_dbgmsg("cli_unzip: lh - has data desc\n");
if(!ch) {
Expand All @@ -581,6 +583,11 @@ static unsigned int lhdr(fmap_t *map, uint32_t loff,uint32_t zsize, unsigned int
zip+=LH_elen;
zsize-=LH_elen;

if (NULL != file_local_header_size)
*file_local_header_size = zip - lh;
if (NULL != file_local_data_size)
*file_local_data_size = csize;

if (!csize) { /* FIXME: what's used for method0 files? csize or usize? Nothing in the specs, needs testing */
cli_dbgmsg("cli_unzip: lh - skipping empty file\n");
} else {
Expand All @@ -589,6 +596,7 @@ static unsigned int lhdr(fmap_t *map, uint32_t loff,uint32_t zsize, unsigned int
fmap_unneed_off(map, loff, SIZEOF_LH);
return 0;
}

if(LH_flags & F_ENCR) {
if(fmap_need_ptr_once(map, zip, csize))
*ret = zdecrypt(zip, csize, usize, lh, fu, ctx, tmpd, zcb);
Expand Down Expand Up @@ -624,12 +632,19 @@ static unsigned int lhdr(fmap_t *map, uint32_t loff,uint32_t zsize, unsigned int
return zip-lh;
}

static unsigned int chdr(fmap_t *map, uint32_t coff, uint32_t zsize, unsigned int *fu, unsigned int fc, int *ret, cli_ctx *ctx, char *tmpd, struct zip_requests *requests) {
static unsigned int chdr(fmap_t *map, uint32_t coff, uint32_t zsize, unsigned int *fu, unsigned int fc, int *ret, cli_ctx *ctx, char *tmpd, struct zip_requests *requests, uint32_t *file_local_offset, uint32_t *file_local_header_size, uint32_t *file_local_data_size) {
char name[256];
int last = 0;
const uint8_t *ch;
int virus_found = 0;

if (NULL != file_local_offset)
*file_local_offset = 0;
if (NULL != file_local_header_size)
*file_local_header_size = 0;
if (NULL != file_local_data_size)
*file_local_data_size = 0;

if(!(ch = fmap_need_off(map, coff, SIZEOF_CH)) || CH_magic != 0x02014b50) {
if(ch) fmap_unneed_ptr(map, ch, SIZEOF_CH);
cli_dbgmsg("cli_unzip: ch - wrkcomplete\n");
Expand Down Expand Up @@ -674,7 +689,9 @@ static unsigned int chdr(fmap_t *map, uint32_t coff, uint32_t zsize, unsigned in

if (!requests) {
if(CH_off<zsize-SIZEOF_LH) {
lhdr(map, CH_off, zsize-CH_off, fu, fc, ch, ret, ctx, tmpd, 1, zip_scan_cb);
if (NULL != file_local_offset)
*file_local_offset = CH_off;
lhdr(map, CH_off, zsize-CH_off, fu, fc, ch, ret, ctx, tmpd, 1, zip_scan_cb, file_local_header_size, file_local_data_size);
} else cli_dbgmsg("cli_unzip: ch - local hdr out of file\n");
}
else {
Expand All @@ -685,7 +702,7 @@ static unsigned int chdr(fmap_t *map, uint32_t coff, uint32_t zsize, unsigned in
for (i = 0; i < requests->namecnt; ++i) {
cli_dbgmsg("checking for %i: %s\n", i, requests->names[i]);

len = MIN(sizeof(name)-1, requests->namelens[i]);
len = MIN(sizeof(name)-1, requests->namelens[i]);
if (!strncmp(requests->names[i], name, len)) {
requests->match = 1;
requests->found = i;
Expand All @@ -712,6 +729,13 @@ int cli_unzip(cli_ctx *ctx) {
#if HAVE_JSON
int toval = 0;
#endif
int bZipBombDetected = 0;
uint32_t cur_file_local_offset = 0;
uint32_t cur_file_local_header_size = 0;
uint32_t cur_file_local_data_size = 0;
uint32_t prev_file_local_offset = 0;
uint32_t prev_file_local_header_size = 0;
uint32_t prev_file_local_data_size = 0;

cli_dbgmsg("in cli_unzip\n");
fsize = (uint32_t)map->len;
Expand Down Expand Up @@ -744,20 +768,48 @@ int cli_unzip(cli_ctx *ctx) {
}

if(coff) {
uint32_t nOverlappingFiles = 0;

cli_dbgmsg("cli_unzip: central @%x\n", coff);
while((coff=chdr(map, coff, fsize, &fu, fc+1, &ret, ctx, tmpd, NULL))) {
while((coff=chdr(map, coff, fsize, &fu, fc+1, &ret, ctx, tmpd, NULL, &cur_file_local_offset, &cur_file_local_header_size, &cur_file_local_data_size))) {
fc++;
if (ctx->engine->maxfiles && fu>=ctx->engine->maxfiles) {
cli_dbgmsg("cli_unzip: Files limit reached (max: %u)\n", ctx->engine->maxfiles);
ret=CL_EMAXFILES;
}
/*
* Detect overlapping files and zip bombs.
*/
if ((((cur_file_local_offset > prev_file_local_offset) && (cur_file_local_offset < prev_file_local_offset + prev_file_local_header_size + prev_file_local_data_size)) ||
((prev_file_local_offset > cur_file_local_offset) && (prev_file_local_offset < cur_file_local_offset + cur_file_local_header_size + cur_file_local_data_size))) &&
(cur_file_local_header_size + cur_file_local_data_size > 0)) {
/* Overlapping file detected */
nOverlappingFiles++;

cli_dbgmsg("cli_unzip: Overlapping files detected.\n");
cli_dbgmsg(" previous file end: %u\n", prev_file_local_offset + prev_file_local_header_size + prev_file_local_data_size);
cli_dbgmsg(" current file start: %u\n", cur_file_local_offset);
if (ZIP_MAX_NUM_OVERLAPPING_FILES < nOverlappingFiles) {
if (SCAN_HEURISTICS) {
ret = cli_append_virus(ctx, "Heuristics.Zip.OverlappingFiles");
virus_found = 1;
} else {
ret = CL_EFORMAT;
}
bZipBombDetected = 1;
}
}
prev_file_local_offset = cur_file_local_offset;
prev_file_local_header_size = cur_file_local_header_size;
prev_file_local_data_size = cur_file_local_data_size;

#if HAVE_JSON
if (cli_json_timeout_cycle_check(ctx, &toval) != CL_SUCCESS) {
ret=CL_ETIMEOUT;
}
#endif
if (ret != CL_CLEAN) {
if (ret == CL_VIRUS && SCAN_ALLMATCHES) {
if (ret == CL_VIRUS && SCAN_ALLMATCHES && !bZipBombDetected) {
ret = CL_CLEAN;
virus_found = 1;
} else
Expand All @@ -769,7 +821,7 @@ int cli_unzip(cli_ctx *ctx) {
ret = CL_VIRUS;
if(fu<=(fc/4)) { /* FIXME: make up a sane ratio or remove the whole logic */
fc = 0;
while (ret==CL_CLEAN && lhoff<fsize && (coff=lhdr(map, lhoff, fsize-lhoff, &fu, fc+1, NULL, &ret, ctx, tmpd, 1, zip_scan_cb))) {
while (ret==CL_CLEAN && lhoff<fsize && (coff=lhdr(map, lhoff, fsize-lhoff, &fu, fc+1, NULL, &ret, ctx, tmpd, 1, zip_scan_cb, NULL, NULL))) {
fc++;
lhoff+=coff;
if (SCAN_ALLMATCHES && ret == CL_VIRUS) {
Expand Down Expand Up @@ -816,7 +868,7 @@ int unzip_single_internal(cli_ctx *ctx, off_t lhoffl, zip_cb zcb)
return CL_CLEAN;
}

lhdr(map, lhoffl, fsize, &fu, 0, NULL, &ret, ctx, NULL, 0, zcb);
lhdr(map, lhoffl, fsize, &fu, 0, NULL, &ret, ctx, NULL, 0, zcb, NULL, NULL);

return ret;
}
Expand Down Expand Up @@ -886,7 +938,7 @@ int unzip_search(cli_ctx *ctx, fmap_t *map, struct zip_requests *requests)

if(coff) {
cli_dbgmsg("unzip_search: central @%x\n", coff);
while(ret==CL_CLEAN && (coff=chdr(zmap, coff, fsize, NULL, fc+1, &ret, ctx, NULL, requests))) {
while(ret==CL_CLEAN && (coff=chdr(zmap, coff, fsize, NULL, fc+1, &ret, ctx, NULL, requests, NULL, NULL, NULL))) {
if (requests->match) {
ret=CL_VIRUS;
}
Expand Down

0 comments on commit dcd26ea

Please sign in to comment.