src/libical/icalvalue.c,src/test/regression.c - fix an overrun

in icalvalue_decode_ical_string(), with associated test

2 files changed, 49 insertions, 3 deletions

diff --git a/src/libical/icalvalue.c b/src/libical/icalvalue.c
index 2ea72d4d..44541498 100644
--- a/src/libical/icalvalue.c
+++ b/src/libical/icalvalue.c
@@ -1511,8 +1511,8 @@ int icalvalue_decode_ical_string(const char *szText, char *szDecText, int nMaxBu
     if ((szText == 0) || (szDecText == 0))
         return 0;
 
-    buf_sz = strlen(szText);
-    str_p = str = (char *)icalmemory_new_buffer(buf_sz + 1);
+    buf_sz = strlen(szText) + 1;
+    str_p = str = (char *)icalmemory_new_buffer(buf_sz);
 
     if (str_p == 0) {
         return 0;
@@ -1525,11 +1525,14 @@ int icalvalue_decode_ical_string(const char *szText, char *szDecText, int nMaxBu
         } else {
             icalmemory_append_char(&str, &str_p, &buf_sz, *p);
         }
+
+        if (str_p - str > nMaxBufferLen)
+            break;
     }
 
     icalmemory_append_char(&str, &str_p, &buf_sz, '\0');
 
-    if ((int)strlen(str) > nMaxBufferLen) {
+    if ((int)strlen(str) >= nMaxBufferLen) {
         icalmemory_free_buffer(str);
         return 0;
     }
diff --git a/src/test/regression.c b/src/test/regression.c
index f1ae757a..756198e2 100644
--- a/src/test/regression.c
+++ b/src/test/regression.c
@@ -4525,6 +4525,48 @@ void test_timezone_from_builtin(void)
     free(strcomp);
 }
 
+void test_icalvalue_decode_ical_string(void)
+{
+    char buff[12];
+    const char *defvalue, *value;
+
+    /* Without escape characters */
+    defvalue = "xxxxx|VALUE";
+    strcpy(buff, defvalue);
+    value = buff + 6;
+
+    ok("Fails to decode into too small buffer", (icalvalue_decode_ical_string(value, buff, 4) == 0));
+    ok("Buffer not changed", (strcmp(buff, defvalue) == 0));
+    ok("Fails to decode into small buffer (only without nul-terminator)", (icalvalue_decode_ical_string(value, buff, 5) == 0));
+    ok("Buffer not changed", (strcmp(buff, defvalue) == 0));
+    ok("Decodes into large-enough buffer", (icalvalue_decode_ical_string(value, buff, 6) != 0));
+    ok("Properly decoded", (strcmp(buff, value) == 0));
+
+    /* With escape character */
+    defvalue = "xxxxx|a\\\\b!";
+    strcpy(buff, defvalue);
+    value = buff + 6;
+
+    ok("Fails to decode into too small buffer", (icalvalue_decode_ical_string(value, buff, 3) == 0));
+    ok("Buffer not changed", (strcmp(buff, defvalue) == 0));
+    ok("Fails to decode into small buffer (only without nul-terminator)", (icalvalue_decode_ical_string(value, buff, 4) == 0));
+    ok("Buffer not changed", (strcmp(buff, defvalue) == 0));
+    ok("Decodes into large-enough buffer", (icalvalue_decode_ical_string(value, buff, 5) != 0));
+    ok("Properly decoded", (strcmp(buff, "a\\b!") == 0));
+
+    /* With ending escape character, which will be ignored */
+    defvalue = "xxxxx|a\\\\\\";
+    strcpy(buff, defvalue);
+    value = buff + 6;
+
+    ok("Fails to decode into too small buffer", (icalvalue_decode_ical_string(value, buff, 1) == 0));
+    ok("Buffer not changed", (strcmp(buff, defvalue) == 0));
+    ok("Fails to decode into small buffer (only without nul-terminator)", (icalvalue_decode_ical_string(value, buff, 2) == 0));
+    ok("Buffer not changed", (strcmp(buff, defvalue) == 0));
+    ok("Decodes into large-enough buffer", (icalvalue_decode_ical_string(value, buff, 3) != 0));
+    ok("Properly decoded", (strcmp(buff, "a\\") == 0));
+}
+
 int main(int argc, char *argv[])
 {
 #if !defined(HAVE_UNISTD_H)
@@ -4662,6 +4704,7 @@ int main(int argc, char *argv[])
     test_run("Test string_to_kind", test_string_to_kind, do_test, do_header);
     test_run("Test set DATE/DATE-TIME VALUE", test_set_date_datetime_value, do_test, do_header);
     test_run("Test timezone from builtin", test_timezone_from_builtin, do_test, do_header);
+    test_run("Test icalvalue_decode_ical_string", test_icalvalue_decode_ical_string, do_test, do_header);
 
     /** OPTIONAL TESTS go here... **/