Skip to content

Commit

Permalink
2287 - fix ole2 vba temp file leak
Browse files Browse the repository at this point in the history
Previous behaviour would remove temp files by deleting the subdirectory
This caused issues in cases (on Windows) where subdirectories aren't created
due to performance concerns

This commit removes tempfiles individually if keeptemp is off

Original patch authored by Thomas Vy
  • Loading branch information
Mickey Sola committed Apr 17, 2023
1 parent a5f136d commit ebcd99e
Show file tree
Hide file tree
Showing 4 changed files with 26 additions and 11 deletions.
21 changes: 19 additions & 2 deletions libclamav/scanners.c
Expand Up @@ -1633,6 +1633,7 @@ static cl_error_t cli_ole2_tempdir_scan_vba_new(const char *dir, cli_ctx *ctx, s
char path[PATH_MAX];
char filename[PATH_MAX];
int tempfd = -1;
char * tempfile = NULL;

if (CL_SUCCESS != (ret = uniq_get(U, "dir", 3, &hash, &hashcnt))) {
cli_dbgmsg("cli_ole2_tempdir_scan_vba_new: uniq_get('dir') failed with ret code (%d)!\n", ret);
Expand All @@ -1649,7 +1650,7 @@ static cl_error_t cli_ole2_tempdir_scan_vba_new(const char *dir, cli_ctx *ctx, s

if (CL_SUCCESS == find_file(filename, dir, path, sizeof(path))) {
cli_dbgmsg("cli_ole2_tempdir_scan_vba_new: Found dir file: %s\n", path);
if ((ret = cli_vba_readdir_new(ctx, path, U, hash, hashcnt, &tempfd, has_macros)) != CL_SUCCESS) {
if ((ret = cli_vba_readdir_new(ctx, path, U, hash, hashcnt, &tempfd, has_macros, &tempfile)) != CL_SUCCESS) {
// FIXME: Since we only know the stream name of the OLE2 stream, but not its path inside the
// OLE2 archive, we don't know if we have the right file. The only thing we can do is
// iterate all of them until one succeeds.
Expand Down Expand Up @@ -1690,9 +1691,17 @@ static cl_error_t cli_ole2_tempdir_scan_vba_new(const char *dir, cli_ctx *ctx, s
if (CL_SUCCESS != ret) {
goto done;
}

close(tempfd);
tempfd = -1;

if (tempfile) {
if (!ctx->engine->keeptmp) {
remove(tempfile);
}
free(tempfile);
tempfile = NULL;
}
}

hashcnt--;
Expand All @@ -1704,6 +1713,14 @@ static cl_error_t cli_ole2_tempdir_scan_vba_new(const char *dir, cli_ctx *ctx, s
tempfd = -1;
}

if (tempfile) {
if (!ctx->engine->keeptmp) {
remove(tempfile);
}
free(tempfile);
tempfile = NULL;
}

return ret;
}

Expand Down
12 changes: 4 additions & 8 deletions libclamav/vba_extract.c
Expand Up @@ -358,7 +358,7 @@ static size_t vba_normalize(unsigned char *buffer, size_t size)
* Read a VBA project in an OLE directory.
* Contrary to cli_vba_readdir, this function uses the dir file to locate VBA modules.
*/
cl_error_t cli_vba_readdir_new(cli_ctx *ctx, const char *dir, struct uniq *U, const char *hash, uint32_t which, int *tempfd, int *has_macros)
cl_error_t cli_vba_readdir_new(cli_ctx *ctx, const char *dir, struct uniq *U, const char *hash, uint32_t which, int *tempfd, int *has_macros, char** tempfile)
{
cl_error_t ret = CL_SUCCESS;
char fullname[1024];
Expand All @@ -367,15 +367,14 @@ cl_error_t cli_vba_readdir_new(cli_ctx *ctx, const char *dir, struct uniq *U, co
size_t data_len;
size_t data_offset;
const char *stream_name = NULL;
char *tempfile = NULL;
uint16_t codepage = CODEPAGE_ISO8859_1;
unsigned i;
char *mbcs_name = NULL, *utf16_name = NULL;
size_t mbcs_name_size = 0, utf16_name_size = 0;
unsigned char *module_data = NULL, *module_data_utf8 = NULL;
size_t module_data_size = 0, module_data_utf8_size = 0;

if (dir == NULL || hash == NULL || tempfd == NULL || has_macros == NULL) {
if (dir == NULL || hash == NULL || tempfd == NULL || has_macros == NULL || tempfile == NULL) {
return CL_EARG;
}

Expand All @@ -398,12 +397,12 @@ cl_error_t cli_vba_readdir_new(cli_ctx *ctx, const char *dir, struct uniq *U, co

*has_macros = *has_macros + 1;

if ((ret = cli_gentempfd_with_prefix(ctx->sub_tmpdir, "vba_project", &tempfile, tempfd)) != CL_SUCCESS) {
if ((ret = cli_gentempfd_with_prefix(ctx->sub_tmpdir, "vba_project", tempfile, tempfd)) != CL_SUCCESS) {
cli_warnmsg("vba_readdir_new: VBA project cannot be dumped to file\n");
goto done;
}

cli_dbgmsg("Dumping VBA project from dir %s to file %s\n", fullname, tempfile);
cli_dbgmsg("Dumping VBA project from dir %s to file %s\n", fullname, *tempfile);

#define CLI_WRITEN(msg, size) \
do { \
Expand Down Expand Up @@ -1315,9 +1314,6 @@ cl_error_t cli_vba_readdir_new(cli_ctx *ctx, const char *dir, struct uniq *U, co
if (stream_name) {
free((void *)stream_name);
}
if (tempfile) {
free(tempfile);
}
if (ret != CL_SUCCESS && *tempfd >= 0) {
close(*tempfd);
*tempfd = -1;
Expand Down
2 changes: 1 addition & 1 deletion libclamav/vba_extract.h
Expand Up @@ -40,7 +40,7 @@ typedef struct vba_project_tag {
} vba_project_t;

vba_project_t *cli_vba_readdir(const char *dir, struct uniq *U, uint32_t which);
cl_error_t cli_vba_readdir_new(cli_ctx *ctx, const char *dir, struct uniq *U, const char *hash, uint32_t which, int *tempfd, int *has_macros);
cl_error_t cli_vba_readdir_new(cli_ctx *ctx, const char *dir, struct uniq *U, const char *hash, uint32_t which, int *tempfd, int *has_macros, char** tempfile);
vba_project_t *cli_wm_readdir(int fd);
void cli_free_vba_project(vba_project_t *vba_project);

Expand Down
2 changes: 2 additions & 0 deletions libclamav/xlm_extract.c
Expand Up @@ -4994,6 +4994,8 @@ cl_error_t cli_extract_xlm_macros_and_images(const char *dir, cli_ctx *ctx, char

FREE(data);

if (tempfile && !ctx->engine->keeptmp)
remove(tempfile);
FREE(tempfile);

return status;
Expand Down

0 comments on commit ebcd99e

Please sign in to comment.