Skip to content
Permalink
Browse files

add anti "../" and leading slash protection to chmextract

  • Loading branch information
kyz committed Oct 20, 2018
1 parent 3a53aa7 commit 7cadd489698be117c47efcadd742651594429e6d
Showing with 27 additions and 123 deletions.
  1. +10 −0 libmspack/ChangeLog
  2. +17 −123 libmspack/src/chmextract.c
@@ -1,3 +1,13 @@
2018-10-20 Stuart Caie <kyzer@cabextract.org.uk>

* src/chmextract.c: add anti "../" and leading slash protection to
chmextract. I'm not pleased about this. All the sample code provided
with libmspack is meant to be simple examples of library use, not
"productised" binaries. Making the "useful" code samples install
as binaries was a mistake. They were never intended to protect you
from unpacking archive files with relative/absolute paths, and I
would prefer that they never will be.

2018-10-17 Stuart Caie <kyzer@cabextract.org.uk>

* cab.h: Make the CAB block input buffer one byte larger, to allow
@@ -25,8 +25,6 @@

mode_t user_umask;

#define FILENAME ".test.chmx"

/**
* Ensures that all directory components in a filepath exist. New directory
* components are created, if necessary.
@@ -51,126 +49,22 @@ static int ensure_filepath(char *path) {
return 1;
}

/**
* Creates a UNIX filename from the internal CAB filename and the given
* parameters.
*
* @param fname the internal CAB filename.
* @param dir a directory path to prepend to the output filename.
* @param lower if non-zero, filename should be made lower-case.
* @param isunix if zero, MS-DOS path seperators are used in the internal
* CAB filename. If non-zero, UNIX path seperators are used.
* @param utf8 if non-zero, the internal CAB filename is encoded in UTF8.
* @return a freshly allocated and created filename, or NULL if there was
* not enough memory.
* @see unix_path_seperators()
*/
static char *create_output_name(unsigned char *fname, unsigned char *dir,
int lower, int isunix, int utf8)
{
unsigned char *p, *name, c, *fe, sep, slash;
unsigned int x;

sep = (isunix) ? '/' : '\\'; /* the path-seperator */
slash = (isunix) ? '\\' : '/'; /* the other slash */

/* length of filename */
x = strlen((char *) fname);
/* UTF8 worst case scenario: tolower() expands all chars from 1 to 3 bytes */
if (utf8) x *= 3;
/* length of output directory */
if (dir) x += strlen((char *) dir);

if (!(name = (unsigned char *) malloc(x + 2))) {
fprintf(stderr, "out of memory!\n");
return NULL;
}

/* start with blank name */
*name = '\0';

/* add output directory if needed */
if (dir) {
strcpy((char *) name, (char *) dir);
strcat((char *) name, "/");
}

/* remove leading slashes */
while (*fname == sep) fname++;

/* copy from fi->filename to new name, converting MS-DOS slashes to UNIX
* slashes as we go. Also lowercases characters if needed.
*/
p = &name[strlen((char *)name)];
fe = &fname[strlen((char *)fname)];

if (utf8) {
/* UTF8 translates two-byte unicode characters into 1, 2 or 3 bytes.
* %000000000xxxxxxx -> %0xxxxxxx
* %00000xxxxxyyyyyy -> %110xxxxx %10yyyyyy
* %xxxxyyyyyyzzzzzz -> %1110xxxx %10yyyyyy %10zzzzzz
*
* Therefore, the inverse is as follows:
* First char:
* 0x00 - 0x7F = one byte char
* 0x80 - 0xBF = invalid
* 0xC0 - 0xDF = 2 byte char (next char only 0x80-0xBF is valid)
* 0xE0 - 0xEF = 3 byte char (next 2 chars only 0x80-0xBF is valid)
* 0xF0 - 0xFF = invalid
*/
do {
if (fname >= fe) {
free(name);
return NULL;
}

/* get next UTF8 char */
if ((c = *fname++) < 0x80) x = c;
else {
if ((c >= 0xC0) && (c < 0xE0)) {
x = (c & 0x1F) << 6;
x |= *fname++ & 0x3F;
}
else if ((c >= 0xE0) && (c < 0xF0)) {
x = (c & 0xF) << 12;
x |= (*fname++ & 0x3F) << 6;
x |= *fname++ & 0x3F;
}
else x = '?';
}

/* whatever is the path seperator -> '/'
* whatever is the other slash -> '\\'
* otherwise, if lower is set, the lowercase version */
if (x == sep) x = '/';
else if (x == slash) x = '\\';
else if (lower) x = (unsigned int) tolower((int) x);

/* integer back to UTF8 */
if (x < 0x80) {
*p++ = (unsigned char) x;
}
else if (x < 0x800) {
*p++ = 0xC0 | (x >> 6);
*p++ = 0x80 | (x & 0x3F);
}
else {
*p++ = 0xE0 | (x >> 12);
*p++ = 0x80 | ((x >> 6) & 0x3F);
*p++ = 0x80 | (x & 0x3F);
}
} while (x);
}
else {
/* regular non-utf8 version */
do {
c = *fname++;
if (c == sep) c = '/';
else if (c == slash) c = '\\';
else if (lower) c = (unsigned char) tolower((int) c);
} while ((*p++ = c));
}
return (char *) name;
char *create_output_name(char *fname) {
char *out, *p;
if ((out = malloc(strlen(fname) + 1))) {
/* remove leading slashes */
while (*fname == '/' || *fname == '\\') fname++;
/* if that removes all characters, just call it "x" */
strcpy(out, (*fname) ? fname : "x");

/* change "../" to "xx/" */
for (p = out; *p; p++) {
if (p[0] == '.' && p[1] == '.' && (p[2] == '/' || p[2] == '\\')) {
p[0] = p[1] = 'x';
}
}
}
return out;
}

static int sortfunc(const void *a, const void *b) {
@@ -205,7 +99,7 @@ int main(int argc, char *argv[]) {
qsort(f, numf, sizeof(struct mschmd_file *), &sortfunc);

for (i = 0; i < numf; i++) {
char *outname = create_output_name((unsigned char *)f[i]->filename,NULL,0,1,0);
char *outname = create_output_name(f[i]->filename);
printf("Extracting %s\n", outname);
ensure_filepath(outname);
if (chmd->extract(chmd, f[i], outname)) {

1 comment on commit 7cadd48

@carnil

This comment has been minimized.

Copy link

carnil commented on 7cadd48 Oct 23, 2018

Issue fixed by this commit got CVE-2018-18586 assigned.

Please sign in to comment.
You can’t perform that action at this time.