From ed05b1054df10a2fbc68000cfdd429daec03a456 Mon Sep 17 00:00:00 2001 From: "Jason A. Donenfeld" Date: Mon, 4 May 2026 18:13:13 +0200 Subject: cgit: truncate all config values at the newline These would be largely invalid anyway (save, I suppose, for Linux file paths that technically can contain new lines). The actual problem is that these get printed back out into cached -- and trusted -- cgitrc files, and if the fields have newlines, the git-config way of less trusted users configuring repos on a shared system can be abused to inject newlines, which then can be used to smuggle global options (including filters, which execute code) into the cached cgitrc. So now, only ever duplicate up to the newline, when dealing with these inputs. Reported-by: Adrian Denkiewicz Signed-off-by: Jason A. Donenfeld --- cgit.c | 90 ++++++++++++++++++++++++++--------------------------------- cgit.h | 4 ++- filter.c | 4 +-- scan-tree.c | 8 +++--- shared.c | 14 ++++++++-- ui-repolist.c | 2 +- 6 files changed, 62 insertions(+), 60 deletions(-) diff --git a/cgit.c b/cgit.c index c33e6fc..ca318e8 100644 --- a/cgit.c +++ b/cgit.c @@ -47,19 +47,19 @@ void cgit_repo_config(struct cgit_repo *repo, const char *name, const char *valu struct string_list_item *item; if (!strcmp(name, "name")) - repo->name = xstrdup(value); + repo->name = strdup_first_line(value); else if (!strcmp(name, "clone-url")) - repo->clone_url = xstrdup(value); + repo->clone_url = strdup_first_line(value); else if (!strcmp(name, "desc")) - repo->desc = xstrdup(value); + repo->desc = strdup_first_line(value); else if (!strcmp(name, "owner")) - repo->owner = xstrdup(value); + repo->owner = strdup_first_line(value); else if (!strcmp(name, "homepage")) - repo->homepage = xstrdup(value); + repo->homepage = strdup_first_line(value); else if (!strcmp(name, "defbranch")) - repo->defbranch = xstrdup(value); + repo->defbranch = strdup_first_line(value); else if (!strcmp(name, "extra-head-content")) - repo->extra_head_content = xstrdup(value); + repo->extra_head_content = strdup_first_line(value); else if (!strcmp(name, "snapshots")) repo->snapshots = ctx.cfg.snapshots & cgit_parse_snapshots_mask(value); else if (!strcmp(name, "enable-blame")) @@ -91,22 +91,22 @@ void cgit_repo_config(struct cgit_repo *repo, const char *name, const char *valu } else if (!strcmp(name, "max-stats")) repo->max_stats = cgit_find_stats_period(value, NULL); else if (!strcmp(name, "module-link")) - repo->module_link= xstrdup(value); + repo->module_link= strdup_first_line(value); else if (skip_prefix(name, "module-link.", &path)) { - item = string_list_append(&repo->submodules, xstrdup(path)); - item->util = xstrdup(value); + item = string_list_append(&repo->submodules, strdup_first_line(path)); + item->util = strdup_first_line(value); } else if (!strcmp(name, "section")) - repo->section = xstrdup(value); + repo->section = strdup_first_line(value); else if (!strcmp(name, "snapshot-prefix")) - repo->snapshot_prefix = xstrdup(value); + repo->snapshot_prefix = strdup_first_line(value); else if (!strcmp(name, "readme") && value != NULL) { if (repo->readme.items == ctx.cfg.readme.items) memset(&repo->readme, 0, sizeof(repo->readme)); - string_list_append(&repo->readme, xstrdup(value)); + string_list_append(&repo->readme, strdup_first_line(value)); } else if (!strcmp(name, "logo") && value != NULL) - repo->logo = xstrdup(value); + repo->logo = strdup_first_line(value); else if (!strcmp(name, "logo-link") && value != NULL) - repo->logo_link = xstrdup(value); + repo->logo_link = strdup_first_line(value); else if (!strcmp(name, "hide")) repo->hide = atoi(value); else if (!strcmp(name, "ignore")) @@ -130,7 +130,7 @@ static void config_cb(const char *name, const char *value) const char *arg; if (!strcmp(name, "section")) - ctx.cfg.section = xstrdup(value); + ctx.cfg.section = strdup_first_line(value); else if (!strcmp(name, "repo.url")) ctx.repo = cgit_add_repo(value); else if (ctx.repo && !strcmp(name, "repo.path")) @@ -138,33 +138,33 @@ static void config_cb(const char *name, const char *value) else if (ctx.repo && skip_prefix(name, "repo.", &arg)) cgit_repo_config(ctx.repo, arg, value); else if (!strcmp(name, "readme")) - string_list_append(&ctx.cfg.readme, xstrdup(value)); + string_list_append(&ctx.cfg.readme, strdup_first_line(value)); else if (!strcmp(name, "root-title")) - ctx.cfg.root_title = xstrdup(value); + ctx.cfg.root_title = strdup_first_line(value); else if (!strcmp(name, "root-desc")) - ctx.cfg.root_desc = xstrdup(value); + ctx.cfg.root_desc = strdup_first_line(value); else if (!strcmp(name, "root-readme")) - ctx.cfg.root_readme = xstrdup(value); + ctx.cfg.root_readme = strdup_first_line(value); else if (!strcmp(name, "css")) - string_list_append(&ctx.cfg.css, xstrdup(value)); + string_list_append(&ctx.cfg.css, strdup_first_line(value)); else if (!strcmp(name, "js")) - string_list_append(&ctx.cfg.js, xstrdup(value)); + string_list_append(&ctx.cfg.js, strdup_first_line(value)); else if (!strcmp(name, "favicon")) - ctx.cfg.favicon = xstrdup(value); + ctx.cfg.favicon = strdup_first_line(value); else if (!strcmp(name, "footer")) - ctx.cfg.footer = xstrdup(value); + ctx.cfg.footer = strdup_first_line(value); else if (!strcmp(name, "head-include")) - ctx.cfg.head_include = xstrdup(value); + ctx.cfg.head_include = strdup_first_line(value); else if (!strcmp(name, "header")) - ctx.cfg.header = xstrdup(value); + ctx.cfg.header = strdup_first_line(value); else if (!strcmp(name, "logo")) - ctx.cfg.logo = xstrdup(value); + ctx.cfg.logo = strdup_first_line(value); else if (!strcmp(name, "logo-link")) - ctx.cfg.logo_link = xstrdup(value); + ctx.cfg.logo_link = strdup_first_line(value); else if (!strcmp(name, "module-link")) - ctx.cfg.module_link = xstrdup(value); + ctx.cfg.module_link = strdup_first_line(value); else if (!strcmp(name, "strict-export")) - ctx.cfg.strict_export = xstrdup(value); + ctx.cfg.strict_export = strdup_first_line(value); else if (!strcmp(name, "virtual-root")) ctx.cfg.virtual_root = ensure_end(value, '/'); else if (!strcmp(name, "noplainemail")) @@ -206,7 +206,7 @@ static void config_cb(const char *name, const char *value) else if (!strcmp(name, "cache-size")) ctx.cfg.cache_size = atoi(value); else if (!strcmp(name, "cache-root")) - ctx.cfg.cache_root = xstrdup(expand_macros(value)); + ctx.cfg.cache_root = strdup_first_line(expand_macros(value)); else if (!strcmp(name, "cache-root-ttl")) ctx.cfg.cache_root_ttl = atoi(value); else if (!strcmp(name, "cache-repo-ttl")) @@ -250,7 +250,7 @@ static void config_cb(const char *name, const char *value) } else if (!strcmp(name, "max-commit-count")) ctx.cfg.max_commit_count = atoi(value); else if (!strcmp(name, "project-list")) - ctx.cfg.project_list = xstrdup(expand_macros(value)); + ctx.cfg.project_list = strdup_first_line(expand_macros(value)); else if (!strcmp(name, "scan-path")) if (ctx.cfg.cache_size) process_cached_repolist(expand_macros(value)); @@ -264,7 +264,7 @@ static void config_cb(const char *name, const char *value) else if (!strcmp(name, "section-from-path")) ctx.cfg.section_from_path = atoi(value); else if (!strcmp(name, "repository-sort")) - ctx.cfg.repository_sort = xstrdup(value); + ctx.cfg.repository_sort = strdup_first_line(value); else if (!strcmp(name, "section-sort")) ctx.cfg.section_sort = atoi(value); else if (!strcmp(name, "source-filter")) @@ -278,19 +278,19 @@ static void config_cb(const char *name, const char *value) else if (!strcmp(name, "side-by-side-diffs")) ctx.cfg.difftype = atoi(value) ? DIFF_SSDIFF : DIFF_UNIFIED; else if (!strcmp(name, "agefile")) - ctx.cfg.agefile = xstrdup(value); + ctx.cfg.agefile = strdup_first_line(value); else if (!strcmp(name, "mimetype-file")) - ctx.cfg.mimetype_file = xstrdup(value); + ctx.cfg.mimetype_file = strdup_first_line(value); else if (!strcmp(name, "renamelimit")) ctx.cfg.renamelimit = atoi(value); else if (!strcmp(name, "remove-suffix")) ctx.cfg.remove_suffix = atoi(value); else if (!strcmp(name, "robots")) - ctx.cfg.robots = xstrdup(value); + ctx.cfg.robots = strdup_first_line(value); else if (!strcmp(name, "clone-prefix")) - ctx.cfg.clone_prefix = xstrdup(value); + ctx.cfg.clone_prefix = strdup_first_line(value); else if (!strcmp(name, "clone-url")) - ctx.cfg.clone_url = xstrdup(value); + ctx.cfg.clone_url = strdup_first_line(value); else if (!strcmp(name, "local-time")) ctx.cfg.local_time = atoi(value); else if (!strcmp(name, "commit-sort")) { @@ -786,13 +786,6 @@ static char *build_snapshot_setting(int bitmap) return strbuf_detach(&result, NULL); } -static char *get_first_line(char *txt) -{ - char *t = xstrdup(txt); - *strchrnul(t, '\n') = '\0'; - return t; -} - static void print_repo(FILE *f, struct cgit_repo *repo) { struct string_list_item *item; @@ -801,11 +794,8 @@ static void print_repo(FILE *f, struct cgit_repo *repo) fprintf(f, "repo.path=%s\n", repo->path); if (repo->owner) fprintf(f, "repo.owner=%s\n", repo->owner); - if (repo->desc) { - char *tmp = get_first_line(repo->desc); - fprintf(f, "repo.desc=%s\n", tmp); - free(tmp); - } + if (repo->desc) + fprintf(f, "repo.desc=%s\n", repo->desc); for_each_string_list_item(item, &repo->readme) { if (item->util) fprintf(f, "repo.readme=%s:%s\n", (char *)item->util, item->string); diff --git a/cgit.h b/cgit.h index 1db3473..7d7ece7 100644 --- a/cgit.h +++ b/cgit.h @@ -393,7 +393,9 @@ extern void cgit_init_filters(void); extern void cgit_prepare_repo_env(struct cgit_repo * repo); -extern int readfile(const char *path, char **buf, size_t *size); +extern int read_first_line(const char *path, char **buf, size_t *size); + +extern char *strdup_first_line(const char *txt); extern char *expand_macros(const char *txt); diff --git a/filter.c b/filter.c index 22b4970..cb01d32 100644 --- a/filter.c +++ b/filter.c @@ -114,7 +114,7 @@ static struct cgit_filter *new_exec_filter(const char *cmd, int argument_count) f = xmalloc(sizeof(*f)); /* We leave argv for now and assign it below. */ - cgit_exec_filter_init(f, xstrdup(cmd), NULL); + cgit_exec_filter_init(f, strdup_first_line(cmd), NULL); f->base.argument_count = argument_count; args_size = (2 + argument_count) * sizeof(char *); f->argv = xmalloc(args_size); @@ -356,7 +356,7 @@ static struct cgit_filter *new_lua_filter(const char *cmd, int argument_count) filter->base.fprintfp = fprintf_lua_filter; filter->base.cleanup = cleanup_lua_filter; filter->base.argument_count = argument_count; - filter->script_file = xstrdup(cmd); + filter->script_file = strdup_first_line(cmd); return &filter->base; } diff --git a/scan-tree.c b/scan-tree.c index 867fcf7..c120efe 100644 --- a/scan-tree.c +++ b/scan-tree.c @@ -133,7 +133,7 @@ static void add_repo(const char *base, struct strbuf *path) strip_suffix_mem(repo->url, &urllen, "/"); repo->url[urllen] = '\0'; } - repo->path = xstrdup(path->buf); + repo->path = strdup_first_line(path->buf); while (!repo->owner) { if ((pwd = getpwuid(st.st_uid)) == NULL) { fprintf(stderr, "Error reading owner-info for %s: %s (%d)\n", @@ -143,13 +143,13 @@ static void add_repo(const char *base, struct strbuf *path) if (pwd->pw_gecos) if ((p = strchr(pwd->pw_gecos, ','))) *p = '\0'; - repo->owner = xstrdup(pwd->pw_gecos ? pwd->pw_gecos : pwd->pw_name); + repo->owner = strdup_first_line(pwd->pw_gecos ? pwd->pw_gecos : pwd->pw_name); } if (repo->desc == cgit_default_repo_desc || !repo->desc) { strbuf_addstr(path, "description"); if (!stat(path->buf, &st)) - readfile(path->buf, &repo->desc, &size); + read_first_line(path->buf, &repo->desc, &size); strbuf_setlen(path, pathlen); } @@ -166,7 +166,7 @@ static void add_repo(const char *base, struct strbuf *path) } if (slash && !n) { *slash = '\0'; - repo->section = xstrdup(rel.buf); + repo->section = strdup_first_line(rel.buf); *slash = '/'; if (starts_with(repo->name, repo->section)) { repo->name += strlen(repo->section); diff --git a/shared.c b/shared.c index cec2c81..8173eb8 100644 --- a/shared.c +++ b/shared.c @@ -52,6 +52,7 @@ struct cgit_repo *cgit_add_repo(const char *url) ret = &cgit_repolist.repos[cgit_repolist.count-1]; memset(ret, 0, sizeof(struct cgit_repo)); ret->url = trim_end(url, '/'); + *strchrnul(ret->url, '\n') = '\0'; ret->name = ret->url; ret->path = NULL; ret->desc = cgit_default_repo_desc; @@ -440,9 +441,10 @@ void cgit_prepare_repo_env(struct cgit_repo * repo) } /* Read the content of the specified file into a newly allocated buffer, - * zeroterminate the buffer and return 0 on success, errno otherwise. + * zeroterminate the buffer, truncate at a new line, and return 0 on success, + * errno otherwise. */ -int readfile(const char *path, char **buf, size_t *size) +int read_first_line(const char *path, char **buf, size_t *size) { int fd, e; struct stat st; @@ -463,10 +465,18 @@ int readfile(const char *path, char **buf, size_t *size) *size = read_in_full(fd, *buf, st.st_size); e = errno; (*buf)[*size] = '\0'; + *strchrnul(*buf, '\n') = '\0'; close(fd); return (*size == st.st_size ? 0 : e); } +char *strdup_first_line(const char *txt) +{ + char *t = xstrdup(txt); + *strchrnul(t, '\n') = '\0'; + return t; +} + static int is_token_char(char c) { return isalnum(c) || c == '_'; diff --git a/ui-repolist.c b/ui-repolist.c index d12e3dd..1b224cf 100644 --- a/ui-repolist.c +++ b/ui-repolist.c @@ -18,7 +18,7 @@ static time_t read_agefile(const char *path) char *buf = NULL; struct strbuf date_buf = STRBUF_INIT; - if (readfile(path, &buf, &size)) { + if (read_first_line(path, &buf, &size)) { free(buf); return 0; } -- cgit v1.2.3