summaryrefslogtreecommitdiffstats
path: root/src/xdg-autostart-generator/xdg-autostart-service.c
diff options
context:
space:
mode:
authorZbigniew Jędrzejewski-Szmek <zbyszek@in.waw.pl>2020-07-07 11:24:36 +0200
committerZbigniew Jędrzejewski-Szmek <zbyszek@in.waw.pl>2020-07-07 12:20:43 +0200
commitd1ca1f7c2ae052e59d0cbe8512e852b9ef059451 (patch)
tree4df512d796ecf2c79c29f49c5bc695f7edc50aa7 /src/xdg-autostart-generator/xdg-autostart-service.c
parentfuzz: add test case that should already be resolved (diff)
downloadsystemd-d1ca1f7c2ae052e59d0cbe8512e852b9ef059451.tar.xz
systemd-d1ca1f7c2ae052e59d0cbe8512e852b9ef059451.zip
xdg-autostart: avoid quadratic behaviour in strv parsing
The fuzzer test case has a giant line with ";;;;;;;;;;;..." which is turned into a strv of empty strings. Unfortunately, when pushing each string, strv_push() needs to walk the whole array, which leads to quadratic behaviour. So let's use greedy_allocation here and also keep location in the string to avoid iterating. build/fuzz-xdg-desktop test/fuzz/fuzz-xdg-desktop/oss-fuzz-22812 51.10s user 0.01s system 99% cpu 51.295 total ↓ build/fuzz-xdg-desktop test/fuzz/fuzz-xdg-desktop/oss-fuzz-22812 0.07s user 0.01s system 96% cpu 0.083 total Fixes https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=22812. Other minor changes: - say "was already defined" instead of "defined multiple times" to make it clear that we're ignoring this second definition, and not all definitions of the key - unescaping needs to be done also for the last entry
Diffstat (limited to 'src/xdg-autostart-generator/xdg-autostart-service.c')
-rw-r--r--src/xdg-autostart-generator/xdg-autostart-service.c68
1 files changed, 48 insertions, 20 deletions
diff --git a/src/xdg-autostart-generator/xdg-autostart-service.c b/src/xdg-autostart-generator/xdg-autostart-service.c
index 0d33c70401..a19d2d7e24 100644
--- a/src/xdg-autostart-generator/xdg-autostart-service.c
+++ b/src/xdg-autostart-generator/xdg-autostart-service.c
@@ -182,6 +182,37 @@ 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) {
+
+ _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 +225,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 +234,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,17 +260,11 @@ 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;
}
@@ -247,11 +272,14 @@ static int xdg_config_parse_strv(
/* Any trailing entry should be ignored if it is empty. */
if (end > start) {
- r = strv_extend(sv, start);
+ r = strv_strndup_unescape_and_push(unit, filename, line,
+ &sv, &n_allocated, &n,
+ start, end);
if (r < 0)
- return log_oom();
+ return r;
}
+ *ret_sv = TAKE_PTR(sv);
return 0;
}