Permalink
Browse files

Fixed a couple of vulnerabilities in gzopen().

A buffer overrun and not escaping the fname parameter.
While I'm at it, add a gzopen test.

Credit for finding them goes to Nick Finco from Google security.

Change-Id: I33264471b12985d1345dd17948258302af8e81dd
  • Loading branch information...
s-kanev committed Jul 19, 2016
1 parent 52f8b59 commit 9673bbd15ba72c9cce15243a462bffb5d9ded9ae
Showing with 92 additions and 59 deletions.
  1. +18 −0 xiosim/BUILD
  2. +43 −59 xiosim/misc.cpp
  3. +31 −0 xiosim/misc_test.cpp
  4. BIN xiosim/test_data/gzopen.gz
View
@@ -397,6 +397,24 @@ cc_library(
],
)
# we don't depend on the misc rule so we can add the extra gzip flag
cc_test(
name = "misc_test",
size = "small",
srcs = [
"misc.cpp",
"misc.h",
"misc_test.cpp",
],
copts = ['-DGZIP_PATH=\\"/bin/gzip\\"'],
data = ["test_data/gzopen.gz"],
deps = [
":core_const",
":catch_impl",
":host",
],
)
cc_library(
name = "stats",
srcs = [
View
@@ -183,81 +183,65 @@ void memswap(void * p1, void * p2, size_t num_bytes)
#ifdef GZIP_PATH
static struct {
const char *type;
const char *ext;
const char *cmd;
const char* type;
const char* ext;
const char* cmd;
} gzcmds[] = {
/* type */ /* extension */ /* command */
{ "r", ".gz", "%s -dc %s" },
{ "rb", ".gz", "%s -dc %s" },
{ "r", ".Z", "%s -dc %s" },
{ "rb", ".Z", "%s -dc %s" },
{ "w", ".gz", "%s > %s" },
{ "wb", ".gz", "%s > %s" }
/* type */ /* extension */ /* command */
{ "r", ".gz", "%s -dc \'%s\'" },
{ "rb", ".gz", "%s -dc \'%s\'" },
{ "r", ".Z", "%s -dc \'%s\'" },
{ "rb", ".Z", "%s -dc \'%s\'" },
{ "w", ".gz", "%s > \'%s\'" },
{ "wb", ".gz", "%s > \'%s\'" }
};
/* same semantics as fopen() except that filenames ending with a ".gz" or ".Z"
will be automagically get compressed */
FILE *
gzopen(const char *fname, const char *type)
{
const char *cmd = NULL;
const char *ext;
FILE *fd;
char str[2048];
/* get the extension */
ext = mystrrchr(fname, '.');
/* check if extension indicates compressed file */
if (ext != NULL && *ext != '\0')
{
for (size_t i=0; i < sizeof(gzcmds) / sizeof(gzcmds[0]); i++)
{
if (!strcmp(gzcmds[i].type, type) && !strcmp(gzcmds[i].ext, ext))
{
cmd = gzcmds[i].cmd;
break;
}
}
FILE* gzopen(const char* fname, const char* type) {
const char* cmd = NULL;
const char* ext;
FILE* fd;
char str[2048];
/* get the extension */
ext = strrchr(fname, '.');
/* check if extension indicates compressed file */
if (ext != NULL && *ext != '\0') {
for (size_t i = 0; i < sizeof(gzcmds) / sizeof(gzcmds[0]); i++) {
if (!strcmp(gzcmds[i].type, type) && !strcmp(gzcmds[i].ext, ext)) {
cmd = gzcmds[i].cmd;
break;
}
}
}
if (!cmd)
{
/* open file */
fd = fopen(fname, type);
}
else
{
/* open pipe to compressor/decompressor */
sprintf(str, cmd, GZIP_PATH, fname);
fd = popen(str, type);
if (!cmd) {
/* open file */
fd = fopen(fname, type);
} else {
/* open pipe to compressor/decompressor */
int len = snprintf(str, sizeof(str), cmd, GZIP_PATH, fname);
if (len >= (int)sizeof(str))
return NULL;
fd = popen(str, type);
}
return fd;
return fd;
}
/* close compressed stream */
void
gzclose(FILE *fd)
{
/* attempt pipe close, otherwise file close */
if (pclose(fd) == -1)
fclose(fd);
void gzclose(FILE* fd) {
/* attempt pipe close, otherwise file close */
if (pclose(fd) == -1)
fclose(fd);
}
#else /* !GZIP_PATH */
FILE *
gzopen(const char *fname, const char *type)
{
return fopen(fname, type);
}
FILE* gzopen(const char* fname, const char* type) { return fopen(fname, type); }
void
gzclose(FILE *fd)
{
fclose(fd);
}
void gzclose(FILE* fd) { fclose(fd); }
#endif /* GZIP_PATH */
View
@@ -0,0 +1,31 @@
#include "catch.hpp"
#include <cstdio>
#include <cstring>
#include <string>
#include "misc.h"
const std::string XIOSIM_PACKAGE_PATH = "xiosim/";
TEST_CASE("gzopen", "gz read") {
std::string path = XIOSIM_PACKAGE_PATH + "test_data/gzopen.gz";
FILE* fd = gzopen(path.c_str(), "r");
REQUIRE(fd != NULL);
char buff[255];
char* res = fgets(buff, sizeof(buff), fd);
REQUIRE(res != NULL);
REQUIRE(strncmp(buff, "0xdecafbad\n", sizeof(buff)) == 0);
gzclose(fd);
}
TEST_CASE("gz overflow", "gz overflow") {
const size_t size = 4 * 1024 * 1024;
char* fname = static_cast<char*>(malloc(size));
memset(fname, 'a', size);
memcpy(fname + size - 4, ".gz", 3);
fname[size - 1] = 0;
FILE* fd = gzopen(fname, "r");
REQUIRE(fd == NULL);
free(fname);
}
View
Binary file not shown.

0 comments on commit 9673bbd

Please sign in to comment.