Merge pull request #16388 from keszybz/xdg-desktop-fuzz-case

xdg-desktop fuzz case

src/sleep/sleep.c

@@ -71,7 +71,7 @@ static int write_hibernate_location_info(const HibernateLocation *hibernate_loca
                         return 0;
                 }
 
-                return log_debug_errno(errno, "/sys/power/resume_offset not writeable: %m");
+                return log_debug_errno(errno, "/sys/power/resume_offset not writable: %m");
         }
 
         xsprintf(offset_str, "%" PRIu64, hibernate_location->offset);

src/test/test-xdg-autostart.c

@@ -67,7 +67,7 @@ static void test_xdg_desktop_parse(unsigned i, const char *s) {
         case 0:
                 assert_se(streq(service->exec_string, "/bin/sleep 100"));
                 assert_se(strv_equal(service->only_show_in, STRV_MAKE("A", "B")));
-                assert_se(strv_equal(service->not_show_in, STRV_MAKE("C", "", "D\\;", "E")));
+                assert_se(strv_equal(service->not_show_in, STRV_MAKE("C", "D\\;", "E")));
                 assert_se(!service->hidden);
                 break;
         case 1:
@@ -81,14 +81,12 @@ static void test_xdg_desktop_parse(unsigned i, const char *s) {
 }
 
 int main(int argc, char *argv[]) {
-        size_t i;
-
         test_setup_logging(LOG_DEBUG);
 
         test_translate_name();
         test_xdg_format_exec_start();
 
-        for (i = 0; i < ELEMENTSOF(xdg_desktop_file); i++)
+        for (size_t i = 0; i < ELEMENTSOF(xdg_desktop_file); i++)
                 test_xdg_desktop_parse(i, xdg_desktop_file[i]);
 
         return 0;

src/xdg-autostart-generator/xdg-autostart-service.c

@@ -182,6 +182,40 @@ static int xdg_config_parse_string(
         return 0;
 }
 
+static int strv_strndup_unescape_and_push(
+                const char *unit,
+                const char *filename,
+                unsigned line,
+                char ***sv,
+                size_t *n_allocated,
+                size_t *n,
+                const char *start,
+                const char *end) {
+
+        if (end == start)
+                return 0;
+
+        _cleanup_free_ char *copy = NULL;
+        int r;
+
+        copy = strndup(start, end - start);
+        if (!copy)
+                return log_oom();
+
+        r = xdg_unescape_string(unit, filename, line, copy);
+        if (r < 0)
+                return r;
+
+        if (!greedy_realloc((void**) sv, n_allocated, *n + 2, sizeof(char*))) /* One extra for NULL */
+                return log_oom();
+
+        (*sv)[*n] = TAKE_PTR(copy);
+        (*sv)[*n + 1] = NULL;
+        (*n)++;
+
+        return 0;
+}
+
 static int xdg_config_parse_strv(
                 const char *unit,
                 const char *filename,
@@ -194,9 +228,7 @@ static int xdg_config_parse_strv(
                 void *data,
                 void *userdata) {
 
-        char ***sv = data;
-        const char *start;
-        const char *end;
+        char ***ret_sv = data;
         int r;
 
         assert(filename);
@@ -205,23 +237,25 @@ static int xdg_config_parse_strv(
         assert(data);
 
         /* XDG does not allow duplicate definitions. */
-        if (*sv) {
-                log_syntax(unit, LOG_ERR, filename, line, 0, "Key %s was defined multiple times, ignoring.", lvalue);
+        if (*ret_sv) {
+                log_syntax(unit, LOG_ERR, filename, line, 0, "Key %s was already defined, ignoring.", lvalue);
                 return 0;
         }
 
-        *sv = strv_new(NULL);
-        if (!*sv)
+        size_t n = 0, n_allocated = 0;
+        _cleanup_strv_free_ char **sv = NULL;
+
+        if (!GREEDY_REALLOC0(sv, n_allocated, 1))
                 return log_oom();
 
         /* We cannot use strv_split because it does not handle escaping correctly. */
-        start = rvalue;
+        const char *start = rvalue, *end;
 
         for (end = start; *end; end++) {
                 if (*end == '\\') {
                         /* Move forward, and ensure it is a valid escape. */
                         end++;
-                        if (strchr("sntr\\;", *end) == NULL) {
+                        if (!strchr("sntr\\;", *end)) {
                                 log_syntax(unit, LOG_ERR, filename, line, 0, "Undefined escape sequence \\%c.", *end);
                                 return 0;
                         }
@@ -229,29 +263,24 @@ static int xdg_config_parse_strv(
                 }
 
                 if (*end == ';') {
-                        _cleanup_free_ char *copy = NULL;
-
-                        copy = strndup(start, end - start);
-                        if (!copy)
-                                return log_oom();
-                        r = xdg_unescape_string(unit, filename, line, copy);
+                        r = strv_strndup_unescape_and_push(unit, filename, line,
+                                                           &sv, &n_allocated, &n,
+                                                           start, end);
                         if (r < 0)
                                 return r;
-                        r = strv_consume(sv, TAKE_PTR(copy));
-                        if (r < 0)
-                                return log_oom();
 
                         start = end + 1;
                 }
         }
 
-        /* Any trailing entry should be ignored if it is empty. */
-        if (end > start) {
-                r = strv_extend(sv, start);
-                if (r < 0)
-                        return log_oom();
-        }
+        /* Handle the trailing entry after the last separator */
+        r = strv_strndup_unescape_and_push(unit, filename, line,
+                                           &sv, &n_allocated, &n,
+                                           start, end);
+        if (r < 0)
+                return r;
 
+        *ret_sv = TAKE_PTR(sv);
         return 0;
 }