changeset 170:0e1e048716e4

fix bug where we failed to pickup the last extended attribute. patch from Emmanuel Andry to fix potential security bug in pst2dii with printf(err).
author Carl Byington <carl@five-ten-sg.com>
date Sun, 22 Mar 2009 14:34:26 -0700
parents 062aa7b7ec22
children 6c1e75bc4cac
files AUTHORS ChangeLog configure.in src/libpst.c src/libpst.h src/pst2dii.cpp.in
diffstat 6 files changed, 82 insertions(+), 73 deletions(-) [+]
line wrap: on
line diff
--- a/AUTHORS	Thu Mar 19 16:46:22 2009 -0700
+++ b/AUTHORS	Sun Mar 22 14:34:26 2009 -0700
@@ -25,6 +25,8 @@
     David Cuadrado <krawek@gmail.com>
     Chris Eagle <cseagle@redshift.com>
     Fridrich Strba <fstrba@novell.com>
+    Emmanuel Andry <eandry@mandriva.org>
+
 
 Testing team:
     Mac OSX - Michael Watson <mike@mikeandgayle.com>
--- a/ChangeLog	Thu Mar 19 16:46:22 2009 -0700
+++ b/ChangeLog	Sun Mar 22 14:34:26 2009 -0700
@@ -1,3 +1,9 @@
+LibPST 0.6.35 (2009-xx-xx)
+===============================
+    * fix bug where we failed to pickup the last extended attribute.
+    * patch from Emmanuel Andry to fix potential security bug in
+      pst2dii with printf(err).
+
 LibPST 0.6.34 (2009-03-19)
 ===============================
     * improve consistency checking when fetching items from the pst file.
--- a/configure.in	Thu Mar 19 16:46:22 2009 -0700
+++ b/configure.in	Sun Mar 22 14:34:26 2009 -0700
@@ -1,5 +1,5 @@
 AC_PREREQ(2.59)
-AC_INIT(libpst,0.6.34,carl@five-ten-sg.com)
+AC_INIT(libpst,0.6.35,carl@five-ten-sg.com)
 AC_CONFIG_SRCDIR([src/libpst.c])
 AC_CONFIG_HEADER([config.h])
 AM_INIT_AUTOMAKE
--- a/src/libpst.c	Thu Mar 19 16:46:22 2009 -0700
+++ b/src/libpst.c	Sun Mar 22 14:34:26 2009 -0700
@@ -518,27 +518,30 @@
 } pst_x_attrib;
 
 
+/** Try to load the extended attributes from the pst file.
+    @return true(1) or false(0) to indicate whether the extended attributes have been loaded
+ */
 int pst_load_extended_attributes(pst_file *pf) {
-    // for PST files this will load up ID2 0x61 and check it's "list" attribute.
+    // for PST files this will load up d_id 0x61 and check it's "assoc_tree" attribute.
     pst_desc_ll *p;
-    pst_mapi_object *na;
+    pst_mapi_object *list;
     pst_id2_ll *id2_head = NULL;
     char *buffer=NULL, *headerbuffer=NULL;
     size_t bsize=0, hsize=0, bptr=0;
     pst_x_attrib xattrib;
     int32_t tint, err=0, x;
-    pst_x_attrib_ll *ptr, *p_head=NULL, *p_sh=NULL, *p_sh2=NULL;
+    pst_x_attrib_ll *ptr, *p_head=NULL;
 
     DEBUG_ENT("pst_loadExtendedAttributes");
     p = pst_getDptr(pf, (uint64_t)0x61);
     if (!p) {
-        DEBUG_WARN(("Cannot find DescID 0x61 for loading the Extended Attributes\n"));
+        DEBUG_WARN(("Cannot find d_id 0x61 for loading the Extended Attributes\n"));
         DEBUG_RET();
         return 0;
     }
 
     if (!p->desc) {
-        DEBUG_WARN(("desc is NULL for item 0x61. Cannot load Extended Attributes\n"));
+        DEBUG_WARN(("descriptor is NULL for d_id 0x61. Cannot load Extended Attributes\n"));
         DEBUG_RET();
         return 0;
     }
@@ -547,49 +550,53 @@
         id2_head = pst_build_id2(pf, p->assoc_tree);
         pst_printID2ptr(id2_head);
     } else {
-        DEBUG_WARN(("Have not been able to fetch any id2 values for item 0x61. Brace yourself!\n"));
+        DEBUG_WARN(("Have not been able to fetch any id2 values for d_id 0x61. Brace yourself!\n"));
     }
 
-    na = pst_parse_block(pf, p->desc->i_id, id2_head);
-    if (!na) {
+    list = pst_parse_block(pf, p->desc->i_id, id2_head);
+    if (!list) {
         DEBUG_WARN(("Cannot process desc block for item 0x61. Not loading extended Attributes\n"));
         pst_free_id2(id2_head);
         DEBUG_RET();
         return 0;
     }
 
-    for (x=0; x < na->count_elements; x++) {
-        if (na->elements[x]->mapi_id == (uint32_t)0x0003) {
-            buffer = na->elements[x]->data;
-            bsize = na->elements[x]->size;
-        } else if (na->elements[x]->mapi_id == (uint32_t)0x0004) {
-            headerbuffer = na->elements[x]->data;
-            hsize = na->elements[x]->size;
+    DEBUG_EMAIL(("look thru d_id 0x61 list of mapi objects\n"));
+    for (x=0; x < list->count_elements; x++) {
+        DEBUG_EMAIL(("#%d - mapi-id: %#x type: %#x length: %#x\n", x, list->elements[x]->mapi_id, list->elements[x]->type, list->elements[x]->size));
+        if (list->elements[x]->data) {
+            DEBUG_HEXDUMPC(list->elements[x]->data, list->elements[x]->size, 0x10);
+        }
+        if (list->elements[x]->mapi_id == (uint32_t)0x0003) {
+            buffer = list->elements[x]->data;
+            bsize  = list->elements[x]->size;
+        } else if (list->elements[x]->mapi_id == (uint32_t)0x0004) {
+            headerbuffer = list->elements[x]->data;
+            hsize        = list->elements[x]->size;
         } else {
             // leave them null
         }
     }
 
     if (!buffer) {
-        pst_free_list(na);
+        pst_free_list(list);
         DEBUG_WARN(("No extended attributes buffer found. Not processing\n"));
         DEBUG_RET();
         return 0;
     }
 
-	xattrib.extended= PST_LE_GET_UINT32(buffer+bptr), bptr += 4;
-	xattrib.type    = PST_LE_GET_UINT16(buffer+bptr), bptr += 2;
-	xattrib.map     = PST_LE_GET_UINT16(buffer+bptr), bptr += 2;
-
-    while (xattrib.type != 0 && bptr < bsize) {
+    while (bptr < bsize) {
+        int err = 0;
+        xattrib.extended= PST_LE_GET_UINT32(buffer+bptr), bptr += 4;
+        xattrib.type    = PST_LE_GET_UINT16(buffer+bptr), bptr += 2;
+        xattrib.map     = PST_LE_GET_UINT16(buffer+bptr), bptr += 2;
         ptr = (pst_x_attrib_ll*) xmalloc(sizeof(*ptr));
         memset(ptr, 0, sizeof(*ptr));
         ptr->type = xattrib.type;
         ptr->map  = xattrib.map+0x8000;
         ptr->next = NULL;
-        DEBUG_INDEX(("xattrib: ext = %#x, type = %#hx, map = %#hx\n",
+        DEBUG_INDEX(("xattrib: ext = %#"PRIx32", type = %#"PRIx16", map = %#"PRIx16"\n",
              xattrib.extended, xattrib.type, xattrib.map));
-        err=0;
         if (xattrib.type & 0x0001) { // if the Bit 1 is set
             // pointer to Unicode field in buffer
             if (xattrib.extended < hsize) {
@@ -602,9 +609,10 @@
                 memcpy(wt, &(headerbuffer[xattrib.extended+sizeof(tint)]), (size_t)tint);
                 ptr->data = pst_wide_to_single(wt, (size_t)tint);
                 free(wt);
-                DEBUG_INDEX(("Read string (converted from UTF-16): %s\n", ptr->data));
+                DEBUG_INDEX(("Mapped attribute %#"PRIx32" to %s\n", ptr->map, ptr->data));
             } else {
                 DEBUG_INDEX(("Cannot read outside of buffer [%i !< %i]\n", xattrib.extended, hsize));
+                err = 1;
             }
             ptr->mytype = PST_MAP_HEADER;
         } else {
@@ -613,16 +621,16 @@
             memset(ptr->data, 0, sizeof(uint32_t));
             *((uint32_t*)ptr->data) = xattrib.extended;
             ptr->mytype = PST_MAP_ATTRIB;
-            DEBUG_INDEX(("Mapped attribute %#x to %#x\n", ptr->map, *((int32_t*)ptr->data)));
+            DEBUG_INDEX(("Mapped attribute %#"PRIx32" to %#"PRIx32"\n", ptr->map, *((uint32_t*)ptr->data)));
         }
 
-        if (err==0) {
+        if (!err) {
             // add it to the list
-            p_sh = p_head;
-            p_sh2 = NULL;
+            pst_x_attrib_ll *p_sh  = p_head;
+            pst_x_attrib_ll *p_sh2 = NULL;
             while (p_sh && ptr->map > p_sh->map) {
                 p_sh2 = p_sh;
-                p_sh = p_sh->next;
+                p_sh  = p_sh->next;
             }
             if (!p_sh2) {
                 // needs to go before first item
@@ -635,16 +643,10 @@
             }
         } else {
             free(ptr);
-            ptr = NULL;
         }
-        memcpy(&xattrib, &(buffer[bptr]), sizeof(xattrib));
-        LE32_CPU(xattrib.extended);
-        LE16_CPU(xattrib.type);
-        LE16_CPU(xattrib.map);
-        bptr += sizeof(xattrib);
     }
     pst_free_id2(id2_head);
-    pst_free_list(na);
+    pst_free_list(list);
     pf->x_head = p_head;
     DEBUG_RET();
     return 1;
@@ -1556,9 +1558,9 @@
                     mo_ptr->elements[x]->mapi_id = *((uint32_t*)mapptr->data);
                     DEBUG_EMAIL(("Mapped attrib %#x to %#x\n", table_rec.type, mo_ptr->elements[x]->mapi_id));
                 } else if (mapptr->mytype == PST_MAP_HEADER) {
-                    DEBUG_EMAIL(("Internet Header mapping found %#x\n", table_rec.type));
+                    DEBUG_EMAIL(("Internet Header mapping found %#"PRIx32" to %s\n", table_rec.type, mapptr->data));
                     mo_ptr->elements[x]->mapi_id = (uint32_t)PST_ATTRIB_HEADER;
-                    mo_ptr->elements[x]->extra = mapptr->data;
+                    mo_ptr->elements[x]->extra   = mapptr->data;
                 }
                 else {
                     DEBUG_WARN(("Missing assertion failure\n"));
@@ -1981,43 +1983,40 @@
         int32_t x;
         for (x=0; x<list->count_elements; x++) {
             int32_t t;
-            pst_item_extra_field *ef;
-            // check here to see if the id is one that is mapped.
             DEBUG_EMAIL(("#%d - mapi-id: %#x type: %#x length: %#x\n", x, list->elements[x]->mapi_id, list->elements[x]->type, list->elements[x]->size));
 
             switch (list->elements[x]->mapi_id) {
                 case PST_ATTRIB_HEADER: // CUSTOM attribute for saying the Extra Headers
-                    if ((list->elements[x]->extra) &&
-                        ((list->elements[x]->type == 0x1f)  ||
-                         (list->elements[x]->type == 0x1e))) {
-                        ef = (pst_item_extra_field*) xmalloc(sizeof(pst_item_extra_field));
+                    if (list->elements[x]->extra) {
+                        pst_item_extra_field *ef = (pst_item_extra_field*) xmalloc(sizeof(pst_item_extra_field));
                         memset(ef, 0, sizeof(pst_item_extra_field));
-                        ef->field_name = (char*) xmalloc(strlen(list->elements[x]->extra)+1);
-                        strcpy(ef->field_name, list->elements[x]->extra);
+                        ef->field_name = strdup(list->elements[x]->extra);
                         LIST_COPY_CSTR(ef->value);
-                        ef->next = item->extra_fields;
-                        item->extra_fields = ef;
-                        DEBUG_EMAIL(("Extra Field - \"%s\" = \"%s\"\n", ef->field_name, ef->value));
-                        if (strcmp(ef->field_name, "content-type") == 0) {
-                            char *p = strstr(ef->value, "charset=\"");
-                            if (p) {
-                                p += 9; // skip over charset="
-                                char *pp = strchr(p, '"');
-                                if (pp) {
-                                    *pp = '\0';
-                                    char *set = strdup(p);
-                                    *pp = '"';
-                                    if (item->body_charset.str) free(item->body_charset.str);
-                                    item->body_charset.str     = set;
-                                    item->body_charset.is_utf8 = 1;
-                                    DEBUG_EMAIL(("body charset %s from content-type extra field\n", set));
+                        if (ef->value) {
+                            ef->next = item->extra_fields;
+                            item->extra_fields = ef;
+                            DEBUG_EMAIL(("Extra Field - \"%s\" = \"%s\"\n", ef->field_name, ef->value));
+                            if (strcmp(ef->field_name, "content-type") == 0) {
+                                char *p = strstr(ef->value, "charset=\"");
+                                if (p) {
+                                    p += 9; // skip over charset="
+                                    char *pp = strchr(p, '"');
+                                    if (pp) {
+                                        *pp = '\0';
+                                        char *set = strdup(p);
+                                        *pp = '"';
+                                        if (item->body_charset.str) free(item->body_charset.str);
+                                        item->body_charset.str     = set;
+                                        item->body_charset.is_utf8 = 1;
+                                        DEBUG_EMAIL(("body charset %s from content-type extra field\n", set));
+                                    }
                                 }
                             }
                         }
-                    }
-                    else {
-                        DEBUG_EMAIL(("What does this mean?\n"));
-                        DEBUG_HEXDUMP(list->elements[x]->data, list->elements[x]->size);
+                        else {
+                            DEBUG_EMAIL(("What does this mean? Internet header %s value\n", list->elements[x]->extra));
+                            DEBUG_HEXDUMP(list->elements[x]->data, list->elements[x]->size);
+                        }
                     }
                     break;
                 case 0x0002: // PR_ALTERNATE_RECIPIENT_ALLOWED
--- a/src/libpst.h	Thu Mar 19 16:46:22 2009 -0700
+++ b/src/libpst.h	Sun Mar 22 14:34:26 2009 -0700
@@ -542,8 +542,10 @@
     pst_x_attrib_ll *x_head;
     pst_block_recorder *block_head;
 
-    int do_read64;              // 0 is 32-bit pst file, pre Outlook 2003;
-                                // 1 is 64-bit pst file, Outlook 2003 and later
+    /** 0 is 32-bit pst file, pre Outlook 2003;
+     *  1 is 64-bit pst file, Outlook 2003 and later
+     */
+    int do_read64;
     uint64_t index1;
     uint64_t index1_back;
     uint64_t index2;
--- a/src/pst2dii.cpp.in	Thu Mar 19 16:46:22 2009 -0700
+++ b/src/pst2dii.cpp.in	Sun Mar 22 14:34:26 2009 -0700
@@ -240,15 +240,15 @@
     while ((p = strchr(l, '&'))) {
         *p = '\0';
         char *err = gdImageStringFTEx(image, &brect[0], color, font_file, sz, 0.0, x_position, y_position, l, &strex);
-        if (err) printf(err);
+        if (err) printf("%s", err);
         x_position += (brect[2]-brect[6]);
         l = p+1;
         err = gdImageStringFTEx(image, &brect[0], color, font_file, sz, 0.0, x_position, y_position, (char*)"&amp;", &strex);
-        if (err) printf(err);
+        if (err) printf("%s", err);
         x_position += (brect[2]-brect[6]);
     }
     char *err = gdImageStringFTEx(image, &brect[0], color, font_file, sz, 0.0, x_position, y_position, l, &strex);
-    if (err) printf(err);
+    if (err) printf("%s", err);
     x_position += (brect[2]-brect[6]);
     col_number += len;
 }
@@ -331,7 +331,7 @@
 
         char line[LINE_SIZE];
         char *err = gdImageStringFTEx(NULL, &brect[0], black, font_file, sz, 0.0, margin, margin, (char*)"LMgqQ", &strex);
-        if (err) printf(err);
+        if (err) printf("%s", err);
         line_height = (brect[3]-brect[7]) * 12/10;
         char_width  = (brect[2]-brect[6]) / 5;
         col_number  = 0;