Add icalparser_ctrl to handle CONTROL characters during parse

RFC 5545 section 3.1 defines CONTROL characters to be byte
values in the decimal ranges [0-8], [10-31] as well as 127.
Of these, all but 10 (LF) and 13 (CR) are not allowed to
be part of any valid content-line produced by the
"contentline" ABNF defined in this section.

However, the current libical parser allows such illegal values
to occur at almost all places within iCalendar and preserves
them in TEXT values as well as non-standard property names.
This causes libical to silently accept invalid iCalendar input
and generate invalid output for such data.

This patch adds the icalparser_ctrl setting to define how to
handle invalid CONTROL characters during parse:

* KEEP: preserve current libical behaviour
* OMIT: silently ignores such characters during parse
* ERROR: rejects the content-line and inserts an error

For backwards-compability, the default setting is KEEP.

ReleaseNotes.txt

@@ -23,6 +23,7 @@ Version 3.1.0 (NOT RELEASED YET):
  * All ical*_new_clone() functions have been deprecated in favour of ical*_clone()
  * Added support for Event Publishing (RFC 9073) and VALARM (RFC 9074) Extensions
  * icaltzutil_get_zone_directory() can use the TZDIR environment to find system zoneinfo
+ * icalparser_ctrl setting defines how to handle invalid CONTROL characters during parsing
  * New publicly available functions:
     + icalrecurrencetype_encode_day
     + icalrecurrencetype_encode_month
@@ -39,6 +40,8 @@ Version 3.1.0 (NOT RELEASED YET):
     + icalcomponent_get_component_name_r
     + ical_set_invalid_rrule_handling_setting
     + ical_get_invalid_rrule_handling_setting
+    + icalparser_get_ctrl
+    + icalparser_set_ctrl
  * Deprecated functions:
     + caldat (replaced by internal function icaldat_int())
     + juldat (replaced by internal function juldat_int())

src/libical/icalparser.c

@@ -28,6 +28,8 @@
 #define MAXIMUM_ALLOWED_MULTIPLE_VALUES 500
 #define MAXIMUM_ALLOWED_ERRORS 100 // Limit the number of errors created by insert_error
 
+static enum icalparser_ctrl icalparser_ctrl_g = ICALPARSER_CTRL_KEEP;
+
 struct icalparser_impl
 {
     int buffer_full;    /* flag indicates that temp is smaller that
@@ -701,6 +703,47 @@ icalcomponent *icalparser_add_line(icalparser *parser, char *line)
         return 0;
     }
 
+    if (icalparser_ctrl_g != ICALPARSER_CTRL_KEEP) {
+	static const unsigned char is_icalctrl[256] = {
+	    1, 1, 1, 1, 1, 1, 1, 1, 1, 0, 0, 1, 1, 0, 1, 1,
+	    1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1,
+	    0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
+	    0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
+	    0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
+	    0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
+	    0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
+	    0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 1,
+	    0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
+	    0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
+	    0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
+	    0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
+	    0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
+	    0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
+	    0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
+	    0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0
+	};
+
+        char *c, *d;
+        for (c = d = line; *c; c++) {
+            if (!is_icalctrl[(unsigned char)*c]) {
+                *d++ = *c;
+            } else if (icalparser_ctrl_g == ICALPARSER_CTRL_OMIT) {
+                // omit CTRL character
+            } else {
+                icalcomponent *tail = pvl_data(pvl_tail(parser->components));
+                if (tail) {
+                    insert_error(
+                            parser, tail, line,
+                            "Content line contains invalid CONTROL characters",
+                            ICAL_XLICERRORTYPE_COMPONENTPARSEERROR);
+                }
+                parser->state = ICALPARSER_ERROR;
+                return 0;
+            }
+        }
+        *d = '\0';
+    }
+
     /* Begin by getting the property name at the start of the line. The
        property name may end up being "BEGIN" or "END" in which case it
        is not really a property, but the marker for the start or end of
@@ -1360,3 +1403,13 @@ icalcomponent *icalparser_parse_string(const char *str)
 
     return c;
 }
+
+enum icalparser_ctrl icalparser_get_ctrl(void)
+{
+    return icalparser_ctrl_g;
+}
+
+void icalparser_set_ctrl(enum icalparser_ctrl ctrl)
+{
+    icalparser_ctrl_g = ctrl;
+}

src/libical/icalparser.h

@@ -303,6 +303,31 @@ LIBICAL_ICAL_EXPORT void icalparser_set_gen_data(icalparser *parser, void *data)
  */
 LIBICAL_ICAL_EXPORT icalcomponent *icalparser_parse_string(const char *str);
 
+/**
+ * @enum icalparser_ctrl
+ * @brief Defines how to handle invalid CONTROL characters in content lines
+ */
+enum icalparser_ctrl {
+    /** Keep CONTROL characters in content-line */
+    ICALPARSER_CTRL_KEEP,
+    /** Omit CONTROL characters from content-line */
+    ICALPARSER_CTRL_OMIT,
+    /** Insert a X-LIC-ERROR instead of content-line */
+    ICALPARSER_CTRL_ERROR
+};
+
+/**
+ * @brief Get the current parser setting how to handle CONTROL characters
+ * @return The current parser setting
+ */
+LIBICAL_ICAL_EXPORT enum icalparser_ctrl icalparser_get_ctrl(void);
+
+/**
+ * @brief Set the parser setting how to handle CONTROL characters
+ * @param ctrl The setting to use
+ */
+LIBICAL_ICAL_EXPORT void icalparser_set_ctrl(enum icalparser_ctrl ctrl);
+
 /***********************************************************************
  * Parser support functions
  ***********************************************************************/

src/test/CMakeLists.txt

@@ -226,3 +226,8 @@ if(NOT WIN32 OR CMAKE_COMPARE_FILES_IGNORE_EOL)
     setprops(icalrecurtest)
   endif()
 endif()
+
+########### next target ###############
+
+set(icalparser_ctrl_test_SRCS icalparser_ctrl_test.c)
+testme(parser_ctrl "${icalparser_ctrl_test_SRCS}")

src/test/icalparser_ctrl_test.c

@@ -0,0 +1,67 @@
+#ifdef HAVE_CONFIG_H
+#include <config.h>
+#endif
+
+#include "libical/ical.h"
+
+static void assert_ctrl(enum icalparser_ctrl ctrl,
+                        const char *data,
+                        const char *want_desc,
+                        int want_xlicerror)
+{
+    icalparser_set_ctrl(ctrl);
+
+    icalcomponent *ical = icalparser_parse_string(data);
+    assert(ical);
+
+    icalcomponent *comp = icalcomponent_get_first_real_component(ical);
+    assert(comp);
+
+    if (want_desc) {
+        assert(!strcmp(want_desc, icalcomponent_get_description(comp)));
+    } else {
+        assert(!icalcomponent_get_first_property(comp, ICAL_DESCRIPTION_PROPERTY));
+    }
+
+    if (want_xlicerror) {
+        assert(icalcomponent_get_first_property(comp, ICAL_XLICERROR_PROPERTY));
+    } else {
+        assert(!icalcomponent_get_first_property(comp, ICAL_XLICERROR_PROPERTY));
+    }
+
+    icalcomponent_free(ical);
+}
+
+int main(int argc, char *argv[])
+{
+    if (argc != 1) {
+        fprintf(stderr, "Usage: %s\n", argv[0]);
+        return 0;
+    }
+
+    // Assert default value
+    assert(icalparser_get_ctrl() == ICALPARSER_CTRL_KEEP);
+
+    // Assert icalparser_ctrl settings
+    static const char *data = ""
+        "BEGIN:VCALENDAR\r\n"
+        "VERSION:2.0\r\n"
+        "PRODID:-//ACME/DesktopCalendar//E\r\n"
+        "CALSCALE:GREGORIAN\r\n"
+        "BEGIN:VEVENT\r\n"
+        "DTSTART:20160929T010000Z\r\n"
+        "DURATION:PT1H\r\n"
+        "UID:40d6fe3c-6a51-489e-823e-3ea22f427a3e\r\n"
+        "CREATED:20150928T125212Z\r\n"
+        "LAST-MODIFIED:20150928T132434Z\r\n"
+        "SUMMARY:test\r\n"
+        "DESCRIPTION:ct\x15rl\r\n" // this contains a CTRL char
+        "END:VEVENT\r\n"
+        "END:VCALENDAR\r\n";
+
+    assert_ctrl(ICALPARSER_CTRL_KEEP, data, "ct\x15rl", 0);
+    assert_ctrl(ICALPARSER_CTRL_OMIT, data, "ctrl", 0);
+    assert_ctrl(ICALPARSER_CTRL_ERROR, data, NULL, 1);
+
+    return 0;
+}