From 58a28334b86fa73b17d64c8c0b3dc027b82653e3 Mon Sep 17 00:00:00 2001 From: TapTap Date: Mon, 14 Sep 2026 16:39:10 +0200 Subject: [PATCH] fix(utils): bound glob matching and line reads Replace the recursive glob matcher with an iterative O(pattern*string) dynamic program. The old recursion explored exponentially many paths for overlapping '*'/'**' wildcards (e.g. '*a*a*...*b' against a long run of 'a'), a CPU DoS reachable from --exclude/--include patterns and .rsync-filter. A differential fuzz against the original matcher confirms identical results. Doc: has_path_traversal() is a lexical '..' check only. Add utils_getdelim_bounded(): a getdelim-style reader that never allocates beyond UTILS_MAX_LINE_LEN, used to cap untrusted list/filter line reads. Tests: pathological glob completes quickly; bounded reader returns EFBIG on an over-long record. --- src/shared/utils.c | 170 +++++++++++++++++++++++++++++--------- src/shared/utils.h | 19 +++++ tests/test_glob.c | 31 +++++++ tests/test_shared_utils.c | 29 +++++++ 4 files changed, 212 insertions(+), 37 deletions(-) diff --git a/src/shared/utils.c b/src/shared/utils.c index 64a5581..07b3d45 100644 --- a/src/shared/utils.c +++ b/src/shared/utils.c @@ -378,59 +378,155 @@ char* output_escape(const char* string, bool eight_bit_output) { return escaped; } +ssize_t utils_getdelim_bounded(FILE* stream, char** line, size_t* cap, int delim, size_t max_len) { + if (!stream || !line || !cap || max_len == 0) { + errno = EINVAL; + return -1; + } + size_t limit = max_len + 1; /* content bytes plus the terminating NUL */ + if (*line == NULL || *cap < 2) { + size_t initial = limit < 256 ? limit : 256; + char* buf = malloc(initial); + if (!buf) + return -1; + free(*line); + *line = buf; + *cap = initial; + } + size_t len = 0; + int c; + while ((c = getc_unlocked(stream)) != EOF) { + if (len >= max_len) { + errno = EFBIG; + return -1; + } + if (len + 2 > *cap) { + size_t new_cap = *cap * 2; + if (new_cap < len + 2) + new_cap = len + 2; + if (new_cap > limit) + new_cap = limit; + char* grown = realloc(*line, new_cap); + if (!grown) + return -1; + *line = grown; + *cap = new_cap; + } + (*line)[len++] = (char)c; + if (c == delim) + break; + } + if (c == EOF && len == 0) + return 0; + (*line)[len] = '\0'; + return (ssize_t)len; +} + /* Match a glob pattern against a string. Supported wildcards: * ? matches any single character except '/'. * * matches any sequence of characters within one path component (no '/'). * ** matches any sequence of characters, including '/' (cross-directory). * slash-star-star-slash is treated as a cross-directory wildcard when it appears between * literals. - */ + * + * The matcher is an iterative O(pattern * string) dynamic program rather than the + * original backtracking recursion: overlapping `*`/`**` wildcards made a pattern + * like `*a*a*a*...*b` run in exponential time against a long run of `a`, a CPU + * denial-of-service vector reachable from a hostile --exclude/--include pattern + * or `.rsync-filter`. The DP reasons over (pattern position, string position) + * so every state is visited once; the transitions below mirror the original + * recursion exactly. */ bool glob_match(const char* pattern, const char* str) { - while (*pattern) { - if (*pattern == '*') { - if (*(pattern + 1) == '*') { - /* globstar: match across directories */ - pattern += 2; - if (*pattern == '\0') - return true; - if (*pattern == '/') - pattern++; - while (*str) { - if (glob_match(pattern, str)) - return true; - str++; + if (!pattern || !str) + return false; + size_t pattern_len = strlen(pattern); + size_t str_len = strlen(str); + if (pattern_len == 0) + return str_len == 0; + /* Defensive work cap: the DP is bounded by pattern*string states, but a + * 64 KiB pattern against a 64 KiB path would still cost billions of steps. + * Treat the pattern as non-matching above the cap instead of burning CPU. */ + if (str_len > (SIZE_MAX / (pattern_len + 1)) - 1) + return false; + if ((pattern_len + 1) * (str_len + 1) > 64u * 1024u * 1024u) + return false; + + size_t row_bytes = str_len + 1; + /* Rows for pattern positions i, i+1, i+2 and i+3 are live at once (the + * globstar transition can skip up to three pattern bytes). Four rotating + * rows keep memory at O(string length); a stack buffer avoids an allocation + * for the common short-leaf case. */ + enum { STACK_ROW = 257 }; + uint8_t stack_rows[4 * STACK_ROW]; + uint8_t* rows = stack_rows; + if (row_bytes > STACK_ROW) { + rows = malloc(4 * row_bytes); + if (!rows) + return false; + } + +#define GLOB_ROW(i) (rows + ((pattern_len - (i)) & 3) * row_bytes) + + /* Base row: pattern position `pattern_len` matches only the string's end. */ + for (size_t j = 0; j <= str_len; j++) + GLOB_ROW(pattern_len)[j] = (j == str_len) ? 1 : 0; + + for (size_t i = pattern_len; i-- > 0;) { + const char pc = pattern[i]; + uint8_t* cur = GLOB_ROW(i); + const uint8_t* next = GLOB_ROW(i + 1); + if (pc == '*') { + if (i + 1 < pattern_len && pattern[i + 1] == '*') { + /* Globstar: skip `**` and an optional following '/', then consume any + * (possibly empty) run of characters -- including '/'. */ + size_t rest = i + 2; + if (rest < pattern_len && pattern[rest] == '/') + rest++; + const uint8_t* rest_row = GLOB_ROW(rest); + for (size_t j = str_len + 1; j-- > 0;) { + bool v = rest_row[j] != 0; + if (!v && j < str_len) + v = cur[j + 1] != 0; + cur[j] = v ? 1 : 0; + } + } else { + /* Single `*`: zero characters, or one non-'/' character. */ + for (size_t j = str_len + 1; j-- > 0;) { + bool v = next[j] != 0; + if (!v && j < str_len && str[j] != '/') + v = cur[j + 1] != 0; + cur[j] = v ? 1 : 0; } - return glob_match(pattern, str); } - /* single *: match within one path component */ - pattern++; - while (*str && *str != '/') { - if (glob_match(pattern, str)) - return true; - str++; + } else if (pc == '?') { + for (size_t j = str_len + 1; j-- > 0;) { + bool v = j < str_len && str[j] != '/' && next[j + 1] != 0; + cur[j] = v ? 1 : 0; } - return glob_match(pattern, str); - } else if (*pattern == '?') { - if (!*str || *str == '/') - return false; - pattern++; - str++; } else { - if (*pattern != *str) { - /* allow literal / ** / rest to match any number of directories */ - if (*pattern == '/' && *(pattern + 1) == '*' && *(pattern + 2) == '*') { - const char* rest = pattern + 3; - if (*rest == '/') + /* Literal: consume an equal character, or -- for a '/' immediately before + * a globstar -- let the '/' match zero directories and continue at `**`. */ + for (size_t j = str_len + 1; j-- > 0;) { + bool v = false; + if (j < str_len && str[j] == pc) { + v = next[j + 1] != 0; + } else if (pc == '/' && i + 2 < pattern_len && pattern[i + 1] == '*' && + pattern[i + 2] == '*') { + size_t rest = i + 3; + if (rest < pattern_len && pattern[rest] == '/') rest++; - return glob_match(rest, str); + v = GLOB_ROW(rest)[j] != 0; } - return false; + cur[j] = v ? 1 : 0; } - pattern++; - str++; } } - return *str == '\0'; + + bool matched = GLOB_ROW(0)[0] != 0; +#undef GLOB_ROW + if (rows != stack_rows) + free(rows); + return matched; } bool format_human_bytes(unsigned long long bytes, char* buffer, size_t buffer_size) { diff --git a/src/shared/utils.h b/src/shared/utils.h index ff83ac0..cda0cd8 100644 --- a/src/shared/utils.h +++ b/src/shared/utils.h @@ -4,7 +4,9 @@ #include "array_list.h" #include #include +#include #include +#include /* Small open-addressing string hash set used to turn quadratic membership * scans into O(path length) exact-match lookups (the --delete keep-set and the @@ -79,6 +81,17 @@ bool path_index_has_descendant(const PathIndex* index, const char* path); char* str_dup(const char* string); char* output_escape(const char* string, bool eight_bit_output); +/* Upper bound on one line/token read from a local list file (--files-from, + * --exclude-from/--include-from, .rsync-filter). Mirrors MAX_STRING_SIZE and + * stops a hostile multi-gigabyte line from forcing unbounded allocation. */ +#define UTILS_MAX_LINE_LEN (64 * 1024) +/* Read one `delim`-terminated record from `stream` into *line (grown as needed + * and NUL-terminated), refusing to consume/allocate more than `max_len` bytes + * of content. Returns the number of bytes stored (delimiter included, matching + * getdelim), 0 at end of file, or -1 on error (errno is EFBIG when the record + * exceeds `max_len`, ENOMEM on allocation failure). *line and *cap are updated + * as the buffer grows and the caller owns *line. */ +ssize_t utils_getdelim_bounded(FILE* stream, char** line, size_t* cap, int delim, size_t max_len); char* path_cat(const char* path1, const char* path2); bool glob_match(const char* pattern, const char* str); /* Result of a bounded extra-file deletion run. */ @@ -148,6 +161,12 @@ const char* utils_get_authorized_root_path(void); * callers guarantee this); this is containment by string, not by resolved * symlinks. Shared by the utils and file secure-walk root confinement. */ bool path_is_within_root(const char* root, const char* path); +/* True when `path` contains a ".." component. This is a purely lexical + * dot-dot check: an absolute path is NOT rejected here, because default + * (non-relative) transfers legitimately put the sender's absolute source path + * on the wire and the receiver re-roots it under the destination with + * path_cat(). Callers that accept a strictly relative path (e.g. batch paths) + * must reject a leading '/' themselves (see utils_valid_batch_path). */ bool has_path_traversal(const char* path); bool utils_valid_batch_path(const char* path); bool format_human_bytes(unsigned long long bytes, char* buffer, size_t buffer_size); diff --git a/tests/test_glob.c b/tests/test_glob.c index ded4bc6..f59016f 100644 --- a/tests/test_glob.c +++ b/tests/test_glob.c @@ -1,7 +1,9 @@ #include "test_glob.h" #include "utils.h" #include "test_utils.h" +#include #include +#include static void test_glob_exact_match() { EXPECT_TRUE(glob_match("foo", "foo")); @@ -76,6 +78,34 @@ static void test_glob_doublestar_mid() { EXPECT_FALSE(glob_match("a/**/b", "a/x/bad")); } +/* The old backtracking matcher explored an exponential number of paths for a + * pattern with many `*` wildcards against a long run that never matches the + * trailing literal. The iterative matcher must stay bounded: 30 `*a` groups + * followed by `b` against ten thousand `a`s is a few hundred thousand states, + * not 2^30 recursion nodes. */ +static void test_glob_pathological_is_bounded() { + char pattern[128]; + size_t pos = 0; + for (int i = 0; i < 30; i++) { + pattern[pos++] = '*'; + pattern[pos++] = 'a'; + } + pattern[pos++] = 'b'; + pattern[pos] = '\0'; + + char* text = malloc(10001); + EXPECT_NOT_NULL(text); + memset(text, 'a', 10000); + text[10000] = '\0'; + + clock_t start = clock(); + EXPECT_FALSE(glob_match(pattern, text)); + double elapsed = (double)(clock() - start) / CLOCKS_PER_SEC; + EXPECT_TRUE(elapsed < 5.0); + + free(text); +} + void test_glob() { test_glob_exact_match(); test_glob_question_mark(); @@ -91,4 +121,5 @@ void test_glob() { test_glob_doublestar_prefix(); test_glob_doublestar_suffix(); test_glob_doublestar_mid(); + test_glob_pathological_is_bounded(); } diff --git a/tests/test_shared_utils.c b/tests/test_shared_utils.c index c89c006..448dd86 100644 --- a/tests/test_shared_utils.c +++ b/tests/test_shared_utils.c @@ -519,9 +519,38 @@ static void test_path_index_semantics() { path_index_free(&empty); } +/* utils_getdelim_bounded must return normal short lines unchanged and refuse an + * over-long record with EFBIG rather than allocating without bound. */ +static void test_getdelim_bounded() { + FILE* fp = tmpfile(); + EXPECT_NOT_NULL(fp); + const char* short_line = "short\n"; + EXPECT_EQ_INT((int)fwrite(short_line, 1, strlen(short_line), fp), (int)strlen(short_line)); + char big[32]; + memset(big, 'x', 20); + big[20] = '\n'; + EXPECT_EQ_INT((int)fwrite(big, 1, 21, fp), 21); + rewind(fp); + + char* line = NULL; + size_t cap = 0; + ssize_t n = utils_getdelim_bounded(fp, &line, &cap, '\n', 64); + EXPECT_EQ_INT((int)n, 6); + EXPECT_EQ_STR(line, "short\n"); + + errno = 0; + n = utils_getdelim_bounded(fp, &line, &cap, '\n', 10); + EXPECT_EQ_INT((int)n, -1); + EXPECT_EQ_INT(errno, EFBIG); + + free(line); + fclose(fp); +} + void test_shared_utils() { test_path_index_bounded(); test_path_index_semantics(); + test_getdelim_bounded(); test_walker_removes_extras_keeps_manifest_and_protected(); test_walker_keeps_nested_manifest_dirs(); test_walker_max_delete_exceeded_deletes_nothing();