# HG changeset patch # User carl # Date 1191365384 25200 # Node ID 183ae993b9ad9696954d1be0731f8b4aeabcbc4f # Parent be6d5329cc0168e705362e9c3caa8dfecbd68a52 security fix for potential buffer overrun in lz decompress diff -r be6d5329cc01 -r 183ae993b9ad AUTHORS --- a/AUTHORS Fri Aug 24 18:57:59 2007 -0700 +++ b/AUTHORS Tue Oct 02 15:49:44 2007 -0700 @@ -1,4 +1,5 @@ Dave Smith +Dave Smith With contributions by: Joseph Nahmias -- bounces @@ -8,3 +9,4 @@ Nigel Horne Chris Hall Stevens Miller + Brad Hards diff -r be6d5329cc01 -r 183ae993b9ad ChangeLog --- a/ChangeLog Fri Aug 24 18:57:59 2007 -0700 +++ b/ChangeLog Tue Oct 02 15:49:44 2007 -0700 @@ -1,3 +1,9 @@ +LibPST 0.5.12 (2007-10-02) +=============================== + + * security fix from Brad Hards for buffer + overruns in liv-zemple decoding for corrupted or malicious pst files. + LibPST 0.5.11 (2007-08-24) =============================== diff -r be6d5329cc01 -r 183ae993b9ad NEWS --- a/NEWS Fri Aug 24 18:57:59 2007 -0700 +++ b/NEWS Tue Oct 02 15:49:44 2007 -0700 @@ -1,5 +1,6 @@ $Id$ +0.5.12 2007-10-02 security fix for possible buffer overruns in liv-zemple decoding 0.5.11 2007-08-24 fix for unitialized variable 0.5.10 2007-08-20 fix yet more valgrind errors, restructure readpst recursive walk, backwards overrun test 0.5.9 2007-08-12 fix more valgrind errors, pst2ldif wrote undefined data diff -r be6d5329cc01 -r 183ae993b9ad configure.in --- a/configure.in Fri Aug 24 18:57:59 2007 -0700 +++ b/configure.in Tue Oct 02 15:49:44 2007 -0700 @@ -1,7 +1,7 @@ AC_INIT(configure.in) AM_CONFIG_HEADER(config.h) -AM_INIT_AUTOMAKE(libpst,0.5.11) +AM_INIT_AUTOMAKE(libpst,0.5.12) AC_PATH_PROGS(BASH, bash) AC_LANG_CPLUSPLUS diff -r be6d5329cc01 -r 183ae993b9ad regression/regression-tests.bash --- a/regression/regression-tests.bash Fri Aug 24 18:57:59 2007 -0700 +++ b/regression/regression-tests.bash Tue Oct 02 15:49:44 2007 -0700 @@ -1,22 +1,24 @@ #!/bin/bash +val="valgrind --leak-check=full" + for i in {1..6}; do rm -rf output$i mkdir output$i done -# ../src/pst2ldif -b 'o=ams-cc.com, c=US' -c 'newPerson' ams.pst >ams.err -# ../src/readpst -cv -o output1 ams.pst -# ../src/readpst -cl -r -o output2 ams.pst -# ../src/readpst -S -o output3 ams.pst -# ../src/readpst -M -o output4 ams.pst -# ../src/readpst -o output5 mbmg.archive.pst +$val ../src/pst2ldif -b 'o=ams-cc.com, c=US' -c 'newPerson' ams.pst >ams.err 2>&1 +$val ../src/readpst -cv -o output1 ams.pst >out1.err 2>&1 +$val ../src/readpst -cl -r -o output2 ams.pst >out2.err 2>&1 +$val ../src/readpst -S -o output3 ams.pst >out3.err 2>&1 +$val ../src/readpst -M -o output4 ams.pst >out4.err 2>&1 +$val ../src/readpst -o output5 mbmg.archive.pst >out5.err 2>&1 +$val ../src/readpst -o output6 test.pst >out6.err 2>&1 - ../src/readpst -o output1 -d dumper ams.pst - ../src/readpstlog -f I dumper >dumperams.log - -# touch /tmp/pam.pst -# ../src/readpst -o output6 -d dumper /tmp/pam.pst -# ../src/readpstlog -f I dumper >dumperpam.log +# ../src/readpst -o output1 -d dumper ams.pst +# ../src/readpstlog -f I dumper >dumperams.log +# +# ../src/readpst -o output6 -d dumper /tmp/test.pst +# ../src/readpstlog -f I dumper >dumpertest.log rm -f dumper diff -r be6d5329cc01 -r 183ae993b9ad src/libpst.c --- a/src/libpst.c Fri Aug 24 18:57:59 2007 -0700 +++ b/src/libpst.c Tue Oct 02 15:49:44 2007 -0700 @@ -1494,11 +1494,17 @@ #define MALLOC_MESSAGESTORE(x) { if (!x->message_store) { x->message_store = (pst_item_message_store*) xmalloc(sizeof(pst_item_message_store)); memset(x->message_store, 0, sizeof(pst_item_message_store));} } #define MALLOC_JOURNAL(x) { if (!x->journal) { x->journal = (pst_item_journal*) xmalloc(sizeof(pst_item_journal)); memset(x->journal, 0, sizeof(pst_item_journal) );} } #define MALLOC_APPOINTMENT(x) { if (!x->appointment) { x->appointment = (pst_item_appointment*) xmalloc(sizeof(pst_item_appointment)); memset(x->appointment, 0, sizeof(pst_item_appointment) );} } -// malloc space and copy the current item's data -- plus one on the size for good luck (and string termination) -#define LIST_COPY(targ, type) { \ - targ = type realloc(targ, list->items[x]->size+1); \ - memset(targ, 0, list->items[x]->size+1); \ +// malloc space and copy the current item's data null terminated +#define LIST_COPY(targ, type) { \ + targ = type realloc(targ, list->items[x]->size+1); \ memcpy(targ, list->items[x]->data, list->items[x]->size); \ + memset(((char*)targ)+list->items[x]->size, 0, 1); \ +} +// malloc space and copy the current item's data and size +#define LIST_COPY_SIZE(targ, type, mysize) { \ + mysize = list->items[x]->size; \ + targ = type realloc(targ, mysize); \ + memcpy(targ, list->items[x]->data, mysize); \ } #define NULL_CHECK(x) { if (!x) { DEBUG_EMAIL(("NULL_CHECK: Null Found\n")); break;} } @@ -2026,7 +2032,7 @@ // it is unknown DEBUG_EMAIL(("RTF Compressed body - ")); MALLOC_EMAIL(item); - LIST_COPY(item->email->rtf_compressed, (char*)); + LIST_COPY_SIZE(item->email->rtf_compressed, (char*), item->email->rtf_compressed_size); DEBUG_EMAIL(("NOT PRINTED\n")); break; case 0x1010: // PR_RTF_SYNC_PREFIX_COUNT diff -r be6d5329cc01 -r 183ae993b9ad src/libpst.h --- a/src/libpst.h Fri Aug 24 18:57:59 2007 -0700 +++ b/src/libpst.h Tue Oct 02 15:49:44 2007 -0700 @@ -177,58 +177,59 @@ typedef struct _pst_item_email { FILETIME *arrival_date; - int32_t autoforward; // 1 = true, 0 = not set, -1 = false - char *body; - char *cc_address; - char *common_name; - int32_t conv_index; - int32_t conversion_prohib; - int32_t delete_after_submit; // 1 = true, 0 = false - int32_t delivery_report; // 1 = true, 0 = false - char *encrypted_body; - int32_t encrypted_body_size; - char *encrypted_htmlbody; - int32_t encrypted_htmlbody_size; - int32_t flag; - char *header; - char *htmlbody; - int32_t importance; - char *in_reply_to; - int32_t message_cc_me; // 1 = true, 0 = false - int32_t message_recip_me; // 1 = true, 0 = false - int32_t message_to_me; // 1 = true, 0 = false - char *messageid; - int32_t orig_sensitivity; - char *outlook_recipient; - char *outlook_recipient2; - char *outlook_sender; - char *outlook_sender_name; - char *outlook_sender2; - int32_t priority; - char *proc_subject; - int32_t read_receipt; - char *recip_access; - char *recip_address; - char *recip2_access; - char *recip2_address; - int32_t reply_requested; - char *reply_to; - char *return_path_address; - int32_t rtf_body_char_count; - int32_t rtf_body_crc; - char *rtf_body_tag; - char *rtf_compressed; - int32_t rtf_in_sync; // 1 = true, 0 = doesn't exist, -1 = false - int32_t rtf_ws_prefix_count; - int32_t rtf_ws_trailing_count; - char *sender_access; - char *sender_address; - char *sender2_access; - char *sender2_address; - int32_t sensitivity; + int32_t autoforward; // 1 = true, 0 = not set, -1 = false + char *body; + char *cc_address; + char *common_name; + int32_t conv_index; + int32_t conversion_prohib; + int32_t delete_after_submit; // 1 = true, 0 = false + int32_t delivery_report; // 1 = true, 0 = false + char *encrypted_body; + int32_t encrypted_body_size; + char *encrypted_htmlbody; + int32_t encrypted_htmlbody_size; + int32_t flag; + char *header; + char *htmlbody; + int32_t importance; + char *in_reply_to; + int32_t message_cc_me; // 1 = true, 0 = false + int32_t message_recip_me; // 1 = true, 0 = false + int32_t message_to_me; // 1 = true, 0 = false + char *messageid; + int32_t orig_sensitivity; + char *outlook_recipient; + char *outlook_recipient2; + char *outlook_sender; + char *outlook_sender_name; + char *outlook_sender2; + int32_t priority; + char *proc_subject; + int32_t read_receipt; + char *recip_access; + char *recip_address; + char *recip2_access; + char *recip2_address; + int32_t reply_requested; + char *reply_to; + char *return_path_address; + int32_t rtf_body_char_count; + int32_t rtf_body_crc; + char *rtf_body_tag; + char *rtf_compressed; + u_int32_t rtf_compressed_size; + int32_t rtf_in_sync; // 1 = true, 0 = doesn't exist, -1 = false + int32_t rtf_ws_prefix_count; + int32_t rtf_ws_trailing_count; + char *sender_access; + char *sender_address; + char *sender2_access; + char *sender2_address; + int32_t sensitivity; FILETIME *sent_date; pst_entryid *sentmail_folder; - char *sentto_address; + char *sentto_address; pst_item_email_subject *subject; } pst_item_email; diff -r be6d5329cc01 -r 183ae993b9ad src/lzfu.c --- a/src/lzfu.c Fri Aug 24 18:57:59 2007 -0700 +++ b/src/lzfu.c Tue Oct 02 15:49:44 2007 -0700 @@ -15,14 +15,6 @@ #include #include -#ifndef _MSC_VER -#include -#endif - -#ifdef _MSC_VER -#define uint32_t unsigned int -#endif - #include "lzfu.h" #define LZFU_COMPRESSED 0x75465a4c @@ -30,28 +22,24 @@ // initital dictionary #define LZFU_INITDICT "{\\rtf1\\ansi\\mac\\deff0\\deftab720{\\fonttbl;}" \ - "{\\f0\\fnil \\froman \\fswiss \\fmodern \\fscrip" \ - "t \\fdecor MS Sans SerifSymbolArialTimes Ne" \ - "w RomanCourier{\\colortbl\\red0\\green0\\blue0" \ - "\r\n\\par \\pard\\plain\\f0\\fs20\\b\\i\\u\\tab" \ - "\\tx" + "{\\f0\\fnil \\froman \\fswiss \\fmodern \\fscrip" \ + "t \\fdecor MS Sans SerifSymbolArialTimes Ne" \ + "w RomanCourier{\\colortbl\\red0\\green0\\blue0" \ + "\r\n\\par \\pard\\plain\\f0\\fs20\\b\\i\\u\\tab" \ + "\\tx" // initial length of dictionary #define LZFU_INITLENGTH 207 // header for compressed rtf typedef struct _lzfuheader { - uint32_t cbSize; - uint32_t cbRawSize; - uint32_t dwMagic; - uint32_t dwCRC; + u_int32_t cbSize; + u_int32_t cbRawSize; + u_int32_t dwMagic; + u_int32_t dwCRC; } lzfuheader; -/** - We always need to add 0x10 to the buffer offset because we need to skip past the header info -*/ - -unsigned char* lzfu_decompress (unsigned char* rtfcomp, size_t *size) { +unsigned char* lzfu_decompress (unsigned char* rtfcomp, u_int32_t compsize, size_t *size) { // the dictionary buffer unsigned char dict[4096]; // the dictionary pointer @@ -62,9 +50,12 @@ unsigned char flags; // temp value for determining the bits in the flag unsigned char flag_mask; - unsigned int i, in_size; + u_int32_t i; unsigned char *out_buf; - unsigned int out_ptr = 0; + u_int32_t out_ptr = 0; + u_int32_t out_size; + u_int32_t in_ptr; + u_int32_t in_size; memcpy(dict, LZFU_INITDICT, LZFU_INITLENGTH); dict_length = LZFU_INITLENGTH; @@ -73,58 +64,61 @@ LE32_CPU(lzfuhdr.cbRawSize); LE32_CPU(lzfuhdr.dwMagic); LE32_CPU(lzfuhdr.dwCRC); - /* printf("total size: %d\n", lzfuhdr.cbSize+4); - printf("raw size : %d\n", lzfuhdr.cbRawSize); - printf("compressed: %s\n", (lzfuhdr.dwMagic == LZFU_COMPRESSED ? "yes" : "no")); - printf("CRC : %#x\n", lzfuhdr.dwCRC); - printf("\n");*/ - out_buf = (unsigned char*)xmalloc(lzfuhdr.cbRawSize+20); //plus 4 cause we have 2x'}' and a \0 - in_size = 0; - // we add plus one here cause when referencing an array, the index is always one less - // (ie, when accessing 2 element array, highest index is [1]) - while (in_size+0x11 < lzfuhdr.cbSize) { - memcpy(&flags, &(rtfcomp[in_size+0x10]), 1); - in_size += 1; - + //printf("total size: %d\n", lzfuhdr.cbSize+4); + //printf("raw size : %d\n", lzfuhdr.cbRawSize); + //printf("compressed: %s\n", (lzfuhdr.dwMagic == LZFU_COMPRESSED ? "yes" : "no")); + //printf("CRC : %#x\n", lzfuhdr.dwCRC); + //printf("\n"); + out_size = lzfuhdr.cbRawSize + 3; // two braces and a null terminator + out_buf = (unsigned char*)xmalloc(out_size); + in_ptr = sizeof(lzfuhdr); + in_size = (lzfuhdr.cbSize < compsize) ? lzfuhdr.cbSize : compsize; + while (in_ptr < in_size) { + flags = rtfcomp[in_ptr++]; flag_mask = 1; - while (flag_mask != 0 && in_size+0x11 < lzfuhdr.cbSize) { + while (flag_mask) { if (flag_mask & flags) { - // read 2 bytes from input - unsigned short int blkhdr, offset, length; - memcpy(&blkhdr, &(rtfcomp[in_size+0x10]), 2); - LE16_CPU(blkhdr); - in_size += 2; - /* swap the upper and lower bytes of blkhdr */ - blkhdr = (((blkhdr&0xFF00)>>8)+ - ((blkhdr&0x00FF)<<8)); - /* the offset is the first 24 bits of the 32 bit value */ - offset = (blkhdr&0xFFF0)>>4; - /* the length of the dict entry are the last 8 bits */ - length = (blkhdr&0x000F)+2; - // add the value we are about to print to the dictionary - for (i=0; i < length; i++) { - unsigned char c1; - c1 = dict[(offset+i)%4096]; - dict[dict_length]=c1; - dict_length = (dict_length+1) % 4096; - out_buf[out_ptr++] = c1; + // two bytes available? + if (in_ptr+1 < in_size) { + // read 2 bytes from input + unsigned short int blkhdr, offset, length; + memcpy(&blkhdr, rtfcomp+in_ptr, 2); + LE16_CPU(blkhdr); + in_ptr += 2; + /* swap the upper and lower bytes of blkhdr */ + blkhdr = (((blkhdr&0xFF00)>>8)+ + ((blkhdr&0x00FF)<<8)); + /* the offset is the first 12 bits of the 16 bit value */ + offset = (blkhdr&0xFFF0)>>4; + /* the length of the dict entry are the last 4 bits */ + length = (blkhdr&0x000F)+2; + // add the value we are about to print to the dictionary + for (i=0; i < length; i++) { + unsigned char c1; + c1 = dict[(offset+i)%4096]; + dict[dict_length]=c1; + dict_length = (dict_length+1) % 4096; + if (out_ptr < out_size) out_buf[out_ptr++] = c1; + } } } else { - // uncompressed chunk (single byte) - char c1 = rtfcomp[in_size+0x10]; - in_size ++; - dict[dict_length] = c1; - dict_length = (dict_length+1)%4096; - out_buf[out_ptr++] = c1; + // one byte available? + if (in_ptr < in_size) { + // uncompressed chunk (single byte) + char c1 = rtfcomp[in_ptr++]; + dict[dict_length] = c1; + dict_length = (dict_length+1)%4096; + if (out_ptr < out_size) out_buf[out_ptr++] = c1; + } } flag_mask <<= 1; } } - // the compressed version doesn't appear to drop the closing braces onto the doc. - // we should do that - out_buf[out_ptr++] = '}'; - out_buf[out_ptr++] = '}'; + // the compressed version doesn't appear to drop the closing + // braces onto the doc, so we do that here. + if (out_ptr < out_size) out_buf[out_ptr++] = '}'; + if (out_ptr < out_size) out_buf[out_ptr++] = '}'; *size = out_ptr; - out_buf[out_ptr++] = '\0'; + if (out_ptr < out_size) out_buf[out_ptr++] = '\0'; return out_buf; } diff -r be6d5329cc01 -r 183ae993b9ad src/lzfu.h --- a/src/lzfu.h Fri Aug 24 18:57:59 2007 -0700 +++ b/src/lzfu.h Tue Oct 02 15:49:44 2007 -0700 @@ -1,4 +1,12 @@ #ifndef LZFU_H #define LZFU_H -unsigned char* lzfu_decompress (unsigned char* rtfcomp, size_t *size); + +#ifdef _MSC_VER +#define u_int32_t unsigned int +#else +#include #endif + +unsigned char* lzfu_decompress (unsigned char* rtfcomp, u_int32_t compsize, size_t *size); + +#endif diff -r be6d5329cc01 -r 183ae993b9ad src/pst2ldif.cpp --- a/src/pst2ldif.cpp Fri Aug 24 18:57:59 2007 -0700 +++ b/src/pst2ldif.cpp Tue Oct 02 15:49:44 2007 -0700 @@ -79,6 +79,19 @@ //////////////////////////////////////////////// +// helper to free all the strings in a set +// +static void free_strings(string_set &s); +static void free_strings(string_set &s) +{ + for (string_set::iterator i=s.begin(); i!=s.end(); i++) { + free(*i); + } + s.clear(); +} + + +//////////////////////////////////////////////// // helper to register a string in a string set // static char* register_string(string_set &s, char *name); @@ -443,6 +456,7 @@ process(d_ptr->child); // do the children of TOPF pst_close(&pstfile); DEBUG_RET(); + free_strings(all_strings); return 0; } diff -r be6d5329cc01 -r 183ae993b9ad src/readpst.c --- a/src/readpst.c Fri Aug 24 18:57:59 2007 -0700 +++ b/src/readpst.c Tue Oct 02 15:49:44 2007 -0700 @@ -75,7 +75,7 @@ int32_t close_seperate_dir(); int32_t mk_seperate_file(struct file_ll *f); char* my_stristr(char *haystack, char *needle); -char* check_filename(char *fname); +void check_filename(char *fname); char* rfc2426_escape(char *str); int32_t chr_count(char *str, char x); char* rfc2425_datetime_format(FILETIME *ft); @@ -478,7 +478,7 @@ } dir = malloc(strlen(fname)+strlen(OUTPUT_KMAIL_DIR_TEMPLATE)+1); sprintf(dir, OUTPUT_KMAIL_DIR_TEMPLATE, fname); - dir = check_filename(dir); + check_filename(dir); if (D_MKDIR(dir)) { //error occured if (errno != EEXIST) { @@ -528,7 +528,7 @@ int x; char *out_name; DEBUG_ENT("mk_recurse_dir"); - dir = check_filename(dir); + check_filename(dir); if (D_MKDIR (dir)) { if (errno != EEXIST) { // not an error because it exists x = errno; @@ -560,24 +560,18 @@ char *mk_seperate_dir(char *dir) { DEBUG_ENT("mk_seperate_dir"); - #if !defined(WIN32) && !defined(__CYGWIN__) - DIR * sdir = NULL; - struct dirent *dirent = NULL; - struct stat *filestat = xmalloc(sizeof(struct stat)); - #endif - char *dir_name = NULL; + size_t dirsize = strlen(dir) + 10; + char dir_name[dirsize]; int x = 0, y = 0; - dir_name = xmalloc(strlen(dir)+10); - do { if (y == 0) - sprintf(dir_name, "%s", dir); + snprintf(dir_name, dirsize, "%s", dir); else - sprintf(dir_name, "%s" SEP_MAIL_FILE_TEMPLATE, dir, y); // enough for 9 digits allocated above + snprintf(dir_name, dirsize, "%s" SEP_MAIL_FILE_TEMPLATE, dir, y); // enough for 9 digits allocated above - dir_name = check_filename(dir_name); + check_filename(dir_name); DEBUG_MAIN(("about to try creating %s\n", dir_name)); if (D_MKDIR(dir_name)) { if (errno != EEXIST) { // if there is an error, and it doesn't already exist @@ -590,7 +584,7 @@ y++; } while (overwrite == 0); - if (chdir (dir_name)) { + if (chdir(dir_name)) { x = errno; DIE(("mk_recurse_dir: Cannot change to directory %s: %s\n", dir, strerror(x))); } @@ -598,12 +592,15 @@ if (overwrite) { // we should probably delete all files from this directory #if !defined(WIN32) && !defined(__CYGWIN__) + DIR * sdir = NULL; + struct dirent *dirent = NULL; + struct stat filestat; if (!(sdir = opendir("./"))) { WARN(("mk_seperate_dir: Cannot open dir \"%s\" for deletion of old contents\n", "./")); } else { while ((dirent = readdir(sdir))) { - if (lstat(dirent->d_name, filestat) != -1) - if (S_ISREG(filestat->st_mode)) { + if (lstat(dirent->d_name, &filestat) != -1) + if (S_ISREG(filestat.st_mode)) { if (unlink(dirent->d_name)) { y = errno; DIE(("mk_seperate_dir: unlink returned error on file %s: %s\n", dirent->d_name, strerror(y))); @@ -642,7 +639,7 @@ sprintf(f->name, SEP_MAIL_FILE_TEMPLATE, f->email_count + name_offset); if (f->output) fclose(f->output); f->output = NULL; - f->name = check_filename(f->name); + check_filename(f->name); if (!(f->output = fopen(f->name, "w"))) { DIE(("mk_seperate_file: Cannot open file to save email \"%s\"\n", f->name)); } @@ -675,7 +672,7 @@ } -char *check_filename(char *fname) { +void check_filename(char *fname) { char *t = fname; DEBUG_ENT("check_filename"); if (!t) { @@ -1119,7 +1116,7 @@ memset(current_attach, 0, sizeof(pst_item_attach)); current_attach->next = item->attach; item->attach = current_attach; - current_attach->data = lzfu_decompress(item->email->rtf_compressed, ¤t_attach->size); + current_attach->data = lzfu_decompress(item->email->rtf_compressed, item->email->rtf_compressed_size, ¤t_attach->size); current_attach->filename2 = xmalloc(strlen(RTF_ATTACH_NAME)+2); strcpy(current_attach->filename2, RTF_ATTACH_NAME); current_attach->mimetype = xmalloc(strlen(RTF_ATTACH_TYPE)+2); @@ -1382,7 +1379,7 @@ char *temp = (char*) xmalloc (strlen(f->name)+10); //enough room for 10 digits sprintf(temp, "%s", f->name); - temp = check_filename(temp); + check_filename(temp); while ((f->output = fopen(temp, "r"))) { DEBUG_MAIN(("need to increase filename because one already exists with that name\n")); DEBUG_MAIN(("- increasing it to %s%d\n", f->name, x)); @@ -1404,7 +1401,7 @@ DEBUG_MAIN(("f->name = %s\nitem->folder_name = %s\n", f->name, item->file_as)); if (mode != MODE_SEPERATE) { - f->name = check_filename(f->name); + check_filename(f->name); if (!(f->output = fopen(f->name, "w"))) { DIE(("create_enter_dir: Could not open file \"%s\" for write\n", f->name)); }