Bug 1553814 - Heap buffer overflow in icalvalue.c icalmemory_strdup_and_dequote. r=philipp
authorluis.merino@x41-dsec.de
Tue, 04 Jun 2019 17:54:21 +0200
changeset 35763 def9f7f3af210dc5ec4474802cded6c249ee4166
parent 35762 dda6d487c69a5f85cb2d34bfae4825060ca43d31
child 35764 383efc5887ecf146ecfb8f5e5853b5869242e27b
push id392
push userclokep@gmail.com
push dateMon, 02 Sep 2019 20:17:19 +0000
reviewersphilipp
bugs1553814
Bug 1553814 - Heap buffer overflow in icalvalue.c icalmemory_strdup_and_dequote. r=philipp
calendar/libical/src/libical/icalvalue.c
--- a/calendar/libical/src/libical/icalvalue.c
+++ b/calendar/libical/src/libical/icalvalue.c
@@ -184,94 +184,99 @@ icalvalue* icalvalue_new_clone(const ica
 	}
     }
 
     return new;
 }
 
 static char* icalmemory_strdup_and_dequote(const char* str)
 {
-    const char* p;
-    char* out = (char*)malloc(sizeof(char) * strlen(str) +1);
-    char* pout;
+    const char *p;
+    char *out = (char *)malloc(sizeof(char) * strlen(str) + 1);
+    char *pout;
+    int wroteNull = 0;
 
-    if (out == 0){
-	return 0;
+    if (out == 0) {
+        return 0;
     }
 
     pout = out;
 
-    for (p = str; *p!=0; p++){
-	
-	if( *p == '\\')
-	{
-	    p++;
-	    switch(*p){
-		case 0:
-		{
-		    *pout = '\0';
-		    break;
+    /* Stop the loop when encountering a terminator in the source string
+       or if a null has been written to the destination. This prevents
+       reading past the end of the source string if the last character
+       is a backslash. */
+    for (p = str; !wroteNull && *p != 0; p++) {
 
-		}
-		case 'n':
-		case 'N':
-		{
-		    *pout = '\n';
-		    break;
-		}
-		case 't':
-		case 'T':
-		{
-		    *pout = '\t';
-		    break;
-		}
-		case 'r':
-		case 'R':
-		{
-		    *pout = '\r';
-		    break;
-		}
-		case 'b':
-		case 'B':
-		{
-		    *pout = '\b';
-		    break;
-		}
-		case 'f':
-		case 'F':
-		{
-		    *pout = '\f';
-		    break;
-		}
-		case ';':
-		case ',':
-		case '"':
-		case '\\':
-		{
-		    *pout = *p;
-		    break;
-		}
-		default:
-		{
-		    *pout = ' ';
-		}		
-	    }
-	} else {
-	    *pout = *p;
-	}
+        if (*p == '\\') {
+            p++;
+            switch (*p) {
+            case 0:
+                {
+                    wroteNull = 1;      //stops iteration so p isn't incremented past the end of str
+                    *pout = '\0';
+                    break;
+                }
+            case 'n':
+            case 'N':
+                {
+                    *pout = '\n';
+                    break;
+                }
+            case 't':
+            case 'T':
+                {
+                    *pout = '\t';
+                    break;
+                }
+            case 'r':
+            case 'R':
+                {
+                    *pout = '\r';
+                    break;
+                }
+            case 'b':
+            case 'B':
+                {
+                    *pout = '\b';
+                    break;
+                }
+            case 'f':
+            case 'F':
+                {
+                    *pout = '\f';
+                    break;
+                }
+            case ';':
+            case ',':
+            case '"':
+            case '\\':
+                {
+                    *pout = *p;
+                    break;
+                }
+            default:
+                {
+                    *pout = ' ';
+                }
+            }
+        } else {
+            *pout = *p;
+        }
 
-	pout++;
-	
+        pout++;
     }
 
     *pout = '\0';
 
     return out;
 }
 
+
+
  /* 
   * Returns a quoted copy of a string
  */
 
 static char* icalmemory_strdup_and_quote(const char* unquoted_str)
 {
     char *str;
     char *str_p;