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

Minor type safety improvements for FMAP reads

Browse Source 
The `fmap_readn()` function returns a size_t.
On error, it returns (size_t)-1. The SIS, SWF, and TNEF parsers were
storing the result as a signed int and then checking if < 0 for the
error case.

Added a CLI_ISCONTAINED_2_0_TO() macro, like the CLI_ISCONTAINED_0_TO()
macro for use when the big buffer offset == 0, to eliminate pointless
warnings.

Fix enum return type for functions in SWF parser.
Errors are enums, not ints! There's a difference.

Fix if-check in SWF parser where we relied on integer overflow for error
checking.
pull/570/head
Micah Snyder 3 years ago
parent 8011786315
commit 1bfb0522cf
5 changed files with 57 additions and 33 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. 16
      libclamav/others.h
  2. 4
      libclamav/sis.c
  3. 54
      libclamav/swf.c
  4. 2
      libclamav/swf.h
  5. 14
      libclamav/tnef.c

16
libclamav/others.h
Unescape View File

@ -130,6 +130,22 @@ extern uint8_t cli_always_gen_section_hash;
(size_t)(sb) + (size_t)(sb_size) >= (size_t)(bb) && \
(size_t)(sb) <= (size_t)(bb) + (size_t)(bb_size))
/*
* CLI_ISCONTAINED_2(bb, bb_size, sb, sb_size) checks if sb (small buffer) is
* within bb (big buffer).
*
* CLI_ISCONTAINED_2 is the same as CLI_ISCONTAINED except that it allows for
* small-buffers with sb_size == 0.
*
* CLI_ISCONTAINED_2_0_TO is the same as CLI_ISCONTAINED_2 except that `bb` is gone
* and assumed ot be zero.
*/
#define CLI_ISCONTAINED_2_0_TO(bb_size, sb, sb_size) \
((size_t)(bb_size) > 0 && \
(size_t)(sb_size) <= (size_t)(bb_size) && \
(size_t)(sb) + (size_t)(sb_size) <= (size_t)(bb_size) && \
(size_t)(sb) <= (size_t)(bb_size))
#define CLI_MAX_ALLOCATION (182 * 1024 * 1024)
#ifdef HAVE_SYS_PARAM_H

4
libclamav/sis.c
Unescape View File

@ -646,10 +646,10 @@ struct SISTREAM {
static inline int getd(struct SISTREAM *s, uint32_t *v)
{
if (s->sleft < 4) {
int nread;
size_t nread;
memcpy(s->buff, s->buff + s->smax - s->sleft, s->sleft);
nread = fmap_readn(s->map, &s->buff[s->sleft], s->pos, BUFSIZ - s->sleft);
if ((nread < 0) || ((s->sleft = s->smax = nread + s->sleft) < 4)) {
if ((nread == (size_t)-1) || ((s->sleft = s->smax = nread + s->sleft) < 4)) {
return 1;
}
s->pos += nread;

54
libclamav/swf.c
Unescape View File

@ -119,19 +119,21 @@ struct swf_file_hdr {
uint32_t filesize;
};
static int scanzws(cli_ctx *ctx, struct swf_file_hdr *hdr)
static cl_error_t scanzws(cli_ctx *ctx, struct swf_file_hdr *hdr)
{
struct CLI_LZMA lz;
unsigned char inbuff[FILEBUFF], outbuff[FILEBUFF];
fmap_t *map = ctx->fmap;
/* strip off header */
off_t offset = 8;
size_t offset = 8;
uint32_t d_insize;
size_t outsize = 8;
int ret, lret;
cl_error_t ret;
int lret;
size_t count;
char *tmpname;
int fd;
size_t n_read;
if ((ret = cli_gentempfd(ctx->sub_tmpdir, &tmpname, &fd)) != CL_SUCCESS) {
cli_errmsg("scanzws: Can't generate temporary file\n");
@ -174,8 +176,8 @@ static int scanzws(cli_ctx *ctx, struct swf_file_hdr *hdr)
}
/* first buffer required for initializing LZMA */
ret = fmap_readn(map, inbuff, offset, FILEBUFF);
if (ret < 0) {
n_read = fmap_readn(map, inbuff, offset, FILEBUFF);
if (n_read == (size_t)-1) {
cli_errmsg("scanzws: Error reading SWF file\n");
close(fd);
if (cli_unlink(tmpname)) {
@ -186,7 +188,7 @@ static int scanzws(cli_ctx *ctx, struct swf_file_hdr *hdr)
return CL_EUNPACK;
}
/* nothing written, likely truncated */
if (!ret) {
if (0 == n_read) {
cli_errmsg("scanzws: possibly truncated file\n");
close(fd);
if (cli_unlink(tmpname)) {
@ -196,12 +198,12 @@ static int scanzws(cli_ctx *ctx, struct swf_file_hdr *hdr)
free(tmpname);
return CL_EFORMAT;
}
offset += ret;
offset += n_read;
memset(&lz, 0, sizeof(lz));
lz.next_in = inbuff;
lz.next_out = outbuff;
lz.avail_in = ret;
lz.avail_in = n_read;
lz.avail_out = FILEBUFF;
lret = cli_LzmaInit(&lz, hdr->filesize);
@ -220,8 +222,8 @@ static int scanzws(cli_ctx *ctx, struct swf_file_hdr *hdr)
if (lz.avail_in == 0) {
lz.next_in = inbuff;
ret = fmap_readn(map, inbuff, offset, FILEBUFF);
if (ret < 0) {
n_read = fmap_readn(map, inbuff, offset, FILEBUFF);
if ((size_t)-1 == n_read) {
cli_errmsg("scanzws: Error reading SWF file\n");
cli_LzmaShutdown(&lz);
close(fd);
@ -232,10 +234,10 @@ static int scanzws(cli_ctx *ctx, struct swf_file_hdr *hdr)
free(tmpname);
return CL_EUNPACK;
}
if (!ret)
if (0 == n_read)
break;
lz.avail_in = ret;
offset += ret;
lz.avail_in = n_read;
offset += n_read;
}
lret = cli_LzmaDecode(&lz);
count = FILEBUFF - lz.avail_out;
@ -299,14 +301,17 @@ static int scanzws(cli_ctx *ctx, struct swf_file_hdr *hdr)
return ret;
}
static int scancws(cli_ctx *ctx, struct swf_file_hdr *hdr)
static cl_error_t scancws(cli_ctx *ctx, struct swf_file_hdr *hdr)
{
z_stream stream;
char inbuff[FILEBUFF], outbuff[FILEBUFF];
fmap_t *map = ctx->fmap;
int offset = 8, ret, zret, zend;
fmap_t *map = ctx->fmap;
size_t offset = 8;
int zret, zend;
cl_error_t ret;
size_t outsize = 8;
size_t count;
size_t n_read;
char *tmpname;
int fd;
@ -350,8 +355,8 @@ static int scancws(cli_ctx *ctx, struct swf_file_hdr *hdr)
do {
if (stream.avail_in == 0) {
stream.next_in = (Bytef *)inbuff;
ret = fmap_readn(map, inbuff, offset, FILEBUFF);
if (ret < 0) {
n_read = fmap_readn(map, inbuff, offset, FILEBUFF);
if (n_read == (size_t)-1) {
cli_errmsg("scancws: Error reading SWF file\n");
close(fd);
inflateEnd(&stream);
@ -362,10 +367,10 @@ static int scancws(cli_ctx *ctx, struct swf_file_hdr *hdr)
free(tmpname);
return CL_EUNPACK;
}
if (!ret)
if (0 == n_read)
break;
stream.avail_in = ret;
offset += ret;
stream.avail_in = n_read;
offset += n_read;
}
zret = inflate(&stream, Z_SYNC_FLUSH);
count = FILEBUFF - stream.avail_out;
@ -442,7 +447,7 @@ static const char *tagname(tag_id id)
return NULL;
}
int cli_scanswf(cli_ctx *ctx)
cl_error_t cli_scanswf(cli_ctx *ctx)
{
struct swf_file_hdr file_hdr;
fmap_t *map = ctx->fmap;
@ -495,7 +500,10 @@ int cli_scanswf(cli_ctx *ctx)
cli_dbgmsg("SWF: FrameSize xMin %u xMax %u yMin %u yMax %u\n", xMin, xMax, yMin, yMax);
}
/* We don't need the value from foo, we're just reading to increment the offset safely. */
GETWORD(foo);
UNUSEDPARAM(foo);
GETWORD(val);
cli_dbgmsg("SWF: Frames total: %d\n", val);
@ -520,7 +528,7 @@ int cli_scanswf(cli_ctx *ctx)
cli_dbgmsg("SWF: Invalid tag length.\n");
return CL_EFORMAT;
}
if ((offset + tag_len) < offset) {
if (tag_len > SIZE_MAX - offset) {
cli_warnmsg("SWF: Tag length too large.\n");
break;
}

2
libclamav/swf.h
Unescape View File

@ -36,7 +36,7 @@
#include "others.h"
int cli_scanswf(cli_ctx *ctx);
cl_error_t cli_scanswf(cli_ctx *ctx);
typedef enum {
TAG_END = 0,

14
libclamav/tnef.c
Unescape View File

@ -268,7 +268,7 @@ tnef_message(fmap_t *map, off_t *pos, uint16_t type, uint16_t tag, int32_t lengt
/*cli_dbgmsg("%lu %lu\n", (long)(offset + length), ftell(fp));*/
if (!CLI_ISCONTAINED_2(0, fsize, offset, length)) {
if (!CLI_ISCONTAINED_2_0_TO(fsize, offset, length)) {
cli_dbgmsg("TNEF: Incorrect length field in tnef_message\n");
return -1;
}
@ -327,13 +327,13 @@ tnef_attachment(fmap_t *map, off_t *pos, uint16_t type, uint16_t tag, int32_t le
todo = length;
while (todo) {
unsigned char buf[BUFSIZ];
int32_t got = fmap_readn(map, buf, *pos, MIN(sizeof(buf), todo));
if (got <= 0)
size_t got = fmap_readn(map, buf, *pos, MIN(sizeof(buf), todo));
if (got == 0 || got == (size_t)-1)
break;
(*pos) += got;
(*pos) += (off_t)got;
fileblobAddData(*fbref, buf, got);
todo -= got;
todo -= (uint32_t)got;
}
break;
default:
@ -344,11 +344,11 @@ tnef_attachment(fmap_t *map, off_t *pos, uint16_t type, uint16_t tag, int32_t le
/*cli_dbgmsg("%lu %lu\n", (long)(offset + length), ftell(fp));*/
if (!CLI_ISCONTAINED_2(0, fsize, (off_t)offset, (off_t)length)) {
if (!CLI_ISCONTAINED_2_0_TO(fsize, offset, length)) {
cli_dbgmsg("TNEF: Incorrect length field in tnef_attachment\n");
return -1;
}
(*pos) = (long)(offset + length); /* shouldn't be needed */
(*pos) = offset + (off_t)length; /* shouldn't be needed */
(*pos) += 2;

Write Preview
Loading…
Cancel
Save