Diff options
| author | Bryce Kwon <bryce@brycekwon.com> | |
|---|---|---|
| committer | Bryce Kwon <bryce@brycekwon.com> | |
| commit | ||
| parent | ||
| tree | ||
| download | ||
Harden the request path, scan and error recovery
| -rw-r--r-- | MANUAL.txt | 41 | |
| -rw-r--r-- | custom/cgitrc | 12 | |
| -rw-r--r-- | source/cache.c | 71 | |
| -rw-r--r-- | source/cache.h | 18 | |
| -rw-r--r-- | source/cgit.c | 177 | |
| -rw-r--r-- | source/cgit.h | 17 | |
| -rw-r--r-- | source/cgit.mk | 28 | |
| -rw-r--r-- | source/cmd.c | 2 | |
| -rw-r--r-- | source/cmd.h | 4 | |
| -rw-r--r-- | source/config.c | 2 | |
| -rw-r--r-- | source/filter.c | 56 | |
| -rw-r--r-- | source/filter.h | 10 | |
| -rw-r--r-- | source/html.c | 32 | |
| -rw-r--r-- | source/html.h | 12 | |
| -rw-r--r-- | source/parsing.c | 16 | |
| -rw-r--r-- | source/parsing.h | 6 | |
| -rw-r--r-- | source/scan-tree.c | 117 | |
| -rw-r--r-- | source/shared.c | 105 | |
| -rw-r--r-- | source/shared.h | 26 | |
| -rwxr-xr-x | tests/t0205-config.sh | 6 | |
| -rwxr-xr-x | tests/t0301-security.sh | 31 |
21 files changed, 565 insertions, 224 deletions
@@ -178,9 +178,9 @@ enable-git-config:: settings. The keys gitweb.owner, gitweb.category, and gitweb.description will map to the cgit keys repo.owner, repo.section, and repo.desc respectively. All git config keys that begin with "cgit." will be mapped - to the corresponding "repo." key in cgit, with the filter keys among - them waiting on "trust-scan-filters". Default value: "0". See also: - scan-path, section-from-path, trust-scan-filters. + to the corresponding "repo." key in cgit, subject to "trust-scan-config" + the way a repository's cgitrc is. Default value: "0". See also: + scan-path, section-from-path, trust-scan-config. enable-gitmodules-links:: Flag which, when set to "1", makes submodule listings derive a link from @@ -538,14 +538,16 @@ trailer-filter:: URL values ships as custom/extensions/link-trailers.lua. Default value: none. See also: "Filter API". -trust-scan-filters:: - Flag which, when set to "1", honours the filter settings in a - repository's own cgitrc file and git config found by "scan-path". Those - files belong to whoever can push to the repository, and a filter is a - command cgit runs, so they are ignored with a warning unless the - repositories under the scan are trusted. Filter settings in the main - cgitrc, the "repo.<filter>" form included, never need this. Default - value: "0". See also: "scan-path", "enable-git-config". +trust-scan-config:: + Flag which, when set to "1", honours every setting in a repository's + own cgitrc file and git config found by "scan-path". Those files belong + to whoever can push to the repository, so without it the settings that + run a command, put raw markup on the page, place a link or read a file + off the disk are ignored with a warning. Those are the filters, + head-content, module-link, logo, logo-link, clone-url and a readme that + names a file rather than a git object. Settings in the main cgitrc, the + "repo.<option>" form included, never need this. Default value: "0". See + also: "scan-path", "enable-git-config". virtual-root:: Url which, if specified, will be used as root for all cgit links. It @@ -559,7 +561,7 @@ Repository settings repo.about-filter:: Override the default about-filter. Default value: <about-filter>. See - also: "Filter API", "trust-scan-filters". + also: "Filter API", "trust-scan-config". repo.branch-sort:: Flag which, when set to "age", enables date ordering in the branch ref @@ -572,7 +574,7 @@ repo.clone-url:: repo.commit-filter:: Override the default commit-filter. Default value: <commit-filter>. See - also: "Filter API", "trust-scan-filters". + also: "Filter API", "trust-scan-config". repo.commit-sort:: Flag which, when set to "date", enables strict date ordering in the @@ -594,7 +596,7 @@ repo.desc:: repo.email-filter:: Override the default email-filter. Default value: <email-filter>. See - also: "Filter API", "trust-scan-filters". + also: "Filter API", "trust-scan-config". repo.enable-blame:: A flag which can be used to override the global setting "enable-blame". @@ -724,11 +726,11 @@ repo.snapshots:: repo.source-filter:: Override the default source-filter. Default value: <source-filter>. See - also: "Filter API", "trust-scan-filters". + also: "Filter API", "trust-scan-config". repo.trailer-filter:: Override the default trailer-filter. Default value: <trailer-filter>. See - also: "Filter API", "trust-scan-filters". + also: "Filter API", "trust-scan-config". repo.url:: The relative url used to access the repository. This must be the first @@ -741,9 +743,10 @@ Repository-specific cgitrc file When the option "scan-path" is used to auto-discover git repositories, cgit will try to parse the file "cgitrc" within any found repository. Such a repo-specific config file may contain any of the repo-specific options described above, except -"repo.url" and "repo.path". The filter options among them are only honoured -when "trust-scan-filters" is set to "1", since the file belongs to whoever can -push to the repository. +"repo.url" and "repo.path". The options that run a command, put raw markup on +the page, place a link or read a file off the disk are only honoured when +"trust-scan-config" is set to "1", since the file belongs to whoever can push +to the repository. Note: the "repo." prefix is dropped from the option names in repo-specific config files, e.g. "repo.desc" becomes "desc". diff --git a/custom/cgitrc b/custom/cgitrc index e6532cf..18c973d 100644 --- a/custom/cgitrc +++ b/custom/cgitrc @@ -265,12 +265,12 @@ enable-http-clone=1 # repository ships under custom/extensions/. The ones written as # /path/to/your-command mark where your own command goes. -# Honour the filter settings in a repository's own cgitrc and git config found -# by scan-path. Those files belong to whoever can push, and a filter is a -# command cgit runs, so leave this off unless the scanned repositories are -# trusted. The repo.* lines below never need it. Values are 0 or 1. Default is -# 0. -trust-scan-filters=0 +# Honour every setting in a repository's own cgitrc and git config found by +# scan-path. Those files belong to whoever can push, so without this the +# settings that run a command, put raw markup on the page, place a link or read +# a file off the disk are ignored. The repo.* lines below never need it. Values +# are 0 or 1. Default is 0. +trust-scan-config=0 # Filter command used to format about-page content. The bundled about-render.lua # renders markdown, man pages and plain text. Value is a command optionally diff --git a/source/cache.c b/source/cache.c index 5c1c808..0c86210 100644 --- a/source/cache.c +++ b/source/cache.c @@ -39,6 +39,7 @@ __attribute__((format (printf,1,2))) static void log_error(const char *format, ...) { va_list args; + va_start(args, format); vfprintf(stderr, format, args); va_end(args); @@ -108,6 +109,7 @@ static int key_fits_slot(const char *key) static int close_slot(struct cache_slot *slot) { int err = 0; + if (slot->cache_fd > 0) { if (close(slot->cache_fd)) err = errno; @@ -169,7 +171,7 @@ static int serve_slot(struct cache_slot *slot) err = print_slot(slot); if (err) - log_error("[cgit] Error printing slot %s: %s (%d)\n", slot->path, strerror(err), err); + log_error("[cgit] Unable to send slot %s: %s (%d)\n", slot->path, strerror(err), err); return err; } @@ -200,6 +202,7 @@ static int is_modified(struct cache_slot *slot) static int close_lock(struct cache_slot *slot) { int err = 0; + if (slot->lock_fd > 0) { if (close(slot->lock_fd)) err = errno; @@ -278,23 +281,40 @@ static int unlock_slot(struct cache_slot *slot, int replace_old_slot) return 0; } -// Only one slot is ever being filled at a time, so a single pointer is enough -// for cache_abandon_fill to find its way back to the client. +// Only one slot is ever being filled at a time, so cache_abandon_fill finds +// it here. static struct cache_slot *slot_being_filled; -void cache_abandon_fill(void) +/* + * Copies the page the lock file holds so far to stdout, which by then is the + * client again. + */ +static void replay_lock_file(struct cache_slot *slot) +{ + off_t off = slot->keylen + 1; + ssize_t got; + + if (lseek(slot->lock_fd, off, SEEK_SET) != off) + return; + while ((got = xread(slot->lock_fd, slot->buf, sizeof(slot->buf))) > 0) { + if (write_in_full(STDOUT_FILENO, slot->buf, got) < 0) + return; + } +} + +int cache_abandon_fill(void) { struct cache_slot *slot = slot_being_filled; if (!slot) - return; + return 0; slot_being_filled = NULL; slot->abandoned = 1; - // The page is sitting in html.c's buffer and in stdio's. Empty both - // while stdout still points at the lock file so the half rendered - // page never reaches the client. - html_flush(); + // What git wrote through stdio joins the lock file first, then the + // file is played back so the visitor gets the page up to the point it + // stopped. The buffer in html.c is the caller's to flush, since a + // caller that could not write it must not try again here. fflush(stdout); if (slot->saved_stdout >= 0) { @@ -302,11 +322,15 @@ void cache_abandon_fill(void) close(slot->saved_stdout); slot->saved_stdout = -1; } + replay_lock_file(slot); unlink(slot->lock_path); + return 1; } -// The page is served from the descriptor either way, so a failed rename only -// costs the next request a render. +/* + * The page is served from the descriptor either way, so a failed rename only + * costs the next request a render. + */ static void publish_slot(struct cache_slot *slot) { int err = unlock_slot(slot, 1); @@ -403,20 +427,21 @@ static int process_slot(struct cache_slot *slot) // and two popular pages sharing one slot evict each other on every // alternating visit. if (!err) - log_error("[cgit] Cache slot %s holds a different key, consider a larger cache-size\n", slot->path); + log_error("[cgit] Cache slot %s holds a different key, consider a larger cache-size\n", + slot->path); // If any part of creating a slot fails the page is still rendered // straight to the client and the caller is told the request succeeded, // because it did. close_slot(slot); if ((err = lock_slot(slot)) != 0) { - log_error("[cgit] Error locking slot %s: %s (%d)\n", slot->lock_path, strerror(err), err); + log_error("[cgit] Unable to lock slot %s: %s (%d)\n", slot->lock_path, strerror(err), err); slot->fn(); return 0; } if ((err = fill_slot(slot)) != 0) { - log_error("[cgit] Error filling slot %s: %s (%d)\n", slot->lock_path, strerror(err), err); + log_error("[cgit] Unable to fill slot %s: %s (%d)\n", slot->lock_path, strerror(err), err); unlock_slot(slot, 0); close_lock(slot); // Rendering again is only right when nothing was delivered, @@ -441,14 +466,16 @@ static int process_slot(struct cache_slot *slot) return err; } -// The result lives in a static buffer and is only good until the next call. -static char *format_time(const char *format, time_t when) +/* + * The result lives in a static buffer and is only good until the next call. + */ +static const char *format_time(const char *format, time_t when) { static char buf[64]; struct tm tm; if (!when) - return NULL; + return "-"; gmtime_r(&when, &tm); strftime(buf, sizeof(buf) - 1, format, &tm); return buf; @@ -529,8 +556,8 @@ int cache_ls(const char *path) struct dirent *ent; int err = 0; // A NULL key leaves open_slot with nothing to compare against, so - // every slot it opens is simply read. - struct cache_slot slot = { NULL }; + // every slot it opens is read. + struct cache_slot slot = { 0 }; struct strbuf slot_path = STRBUF_INIT; size_t prefixlen; char *nul; @@ -543,20 +570,20 @@ int cache_ls(const char *path) dir = opendir(path); if (!dir) { err = errno; - log_error("[cgit] Error opening %s: %s (%d)\n", path, strerror(err), err); + log_error("[cgit] Unable to open %s: %s (%d)\n", path, strerror(err), err); return err; } strbuf_addstr(&slot_path, path); strbuf_ensure_end(&slot_path, '/'); prefixlen = slot_path.len; - while ((ent = readdir(dir)) != NULL) { + while ((ent = readdir(dir))) { if (strlen(ent->d_name) != SLOT_NAME_LEN) continue; strbuf_setlen(&slot_path, prefixlen); strbuf_addstr(&slot_path, ent->d_name); slot.path = slot_path.buf; if ((err = open_slot(&slot)) != 0) { - log_error("[cgit] Error opening %s: %s (%d)\n", slot_path.buf, strerror(err), err); + log_error("[cgit] Unable to open %s: %s (%d)\n", slot_path.buf, strerror(err), err); close_slot(&slot); continue; } diff --git a/source/cache.h b/source/cache.h index a653108..bae283c 100644 --- a/source/cache.h +++ b/source/cache.h @@ -19,18 +19,20 @@ typedef void (*cache_fill_fn)(void); */ extern int cache_process(int size, const char *path, const char *key, int ttl, cache_fill_fn fn); -// Write one line per cache slot to stdout, giving its path, modification -// time, size and key. +/* + * Write one line per cache slot to stdout, giving its path, modification time, + * size and key. + */ extern int cache_ls(const char *path); /* - * Give up on the slot being filled, discarding what has been rendered into it - * and putting stdout back on the client. Anything that ends a request part way - * through rendering has to call this before it writes what the visitor should - * see, because until then stdout is the cache file and the visitor is on - * course to receive nothing at all. Does nothing when no slot is being filled. + * Give up on the slot being filled and put stdout back on the client, after + * sending what was rendered into the slot so far. Anything that ends a request + * part way through rendering has to call this before it writes what the + * visitor should see, because until then stdout is the cache file. Returns + * non-zero when a fill was abandoned and zero when none was in progress. */ -extern void cache_abandon_fill(void); +extern int cache_abandon_fill(void); extern unsigned long cache_hash_str(const char *str); diff --git a/source/cgit.c b/source/cgit.c index 5b26137..00ce97f 100644 --- a/source/cgit.c +++ b/source/cgit.c @@ -46,10 +46,8 @@ // a snapshots mask can carry. #define ALL_SNAPSHOT_FORMATS 0xFF -/* - * The first branch found is the fallback, so a repository whose default branch - * does not exist still has something to show. - */ +// The first branch found is the fallback, so a repository whose default branch +// does not exist still has something to show. struct refmatch { char *wanted; char *first; @@ -76,13 +74,16 @@ static void isolate_git_environment(void) /* * Turn a die from anywhere inside git into a rendered page, since a CGI that * produced no output leaves the visitor with whatever the web server makes of - * it. + * it. Git's message names files and objects on the server, so it goes to the + * log and the visitor gets a fixed page. */ static NORETURN void die_routine(const char *msg, va_list params) { - // The error page abandons any cache fill in progress, so the message - // reaches the visitor rather than the cache file stdout points at. - cgit_vprint_error_page(400, "Bad Request", msg, params); + fputs("[cgit] ", stderr); + vfprintf(stderr, msg, params); + fputc('\n', stderr); + cgit_abort_filters(); + cgit_print_error_page(500, "Internal Server Error", "Unable to complete the request"); exit(0); } @@ -154,7 +155,8 @@ static void prepare_context(void) ctx.env.server_port = getenv("SERVER_PORT"); ctx.env.http_cookie = getenv("HTTP_COOKIE"); ctx.env.http_referer = getenv("HTTP_REFERER"); - ctx.env.content_length = getenv("CONTENT_LENGTH") ? strtoul(getenv("CONTENT_LENGTH"), NULL, 10) : 0; + ctx.env.content_length = + getenv("CONTENT_LENGTH") ? strtoul(getenv("CONTENT_LENGTH"), NULL, 10) : 0; ctx.env.authenticated = 0; ctx.page.mimetype = "text/html"; ctx.page.charset = PAGE_ENCODING; @@ -189,6 +191,7 @@ static void print_version(void) static int cmp_repos(const void *a, const void *b) { const struct cgit_repo *repo_a = a, *repo_b = b; + return strcmp(repo_a->url, repo_b->url); } @@ -213,7 +216,8 @@ static void print_repo(FILE *f, struct cgit_repo *repo) fprintf(f, "repo.url=%s\n", repo->url); fprintf(f, "repo.name=%s\n", repo->name); - fprintf(f, "repo.path=%s\n", repo->path); + if (repo->path) + fprintf(f, "repo.path=%s\n", repo->path); if (repo->owner) fprintf(f, "repo.owner=%s\n", repo->owner); if (repo->desc) @@ -318,7 +322,10 @@ static void parse_args(int argc, const char **argv) ctx.qry.has_oid = 1; } else if (skip_prefix(argv[i], "--ofs=", &arg)) { ctx.qry.ofs = atoi(arg); - } else if (skip_prefix(argv[i], "--scan-tree=", &arg) || skip_prefix(argv[i], "--scan-path=", &arg)) { + } else if ( + skip_prefix(argv[i], "--scan-tree=", &arg) || + skip_prefix(argv[i], "--scan-path=", &arg) + ) { // A repository's snapshots setting is masked with the // global one, and cgitrc has not been read here, so an // empty mask would discard what the repository set. @@ -358,7 +365,7 @@ static int generate_cached_repolist(const char *path, const char *cached_rc) fd = open(locked_rc.buf, O_RDWR | O_CREAT, S_IRUSR | S_IWUSR); if (fd == -1) { err = errno; - fprintf(stderr, "[cgit] Error opening %s: %s (%d)\n", locked_rc.buf, strerror(err), err); + fprintf(stderr, "[cgit] Unable to open %s: %s (%d)\n", locked_rc.buf, strerror(err), err); goto out; } if (fcntl(fd, F_SETLK, &lock) < 0) { @@ -368,7 +375,7 @@ static int generate_cached_repolist(const char *path, const char *cached_rc) // on every request and does deserve one. err = errno; if (err != EACCES && err != EAGAIN) - fprintf(stderr, "[cgit] Error locking %s: %s (%d)\n", locked_rc.buf, strerror(err), err); + fprintf(stderr, "[cgit] Unable to lock %s: %s (%d)\n", locked_rc.buf, strerror(err), err); close(fd); goto out; } @@ -390,7 +397,7 @@ static int generate_cached_repolist(const char *path, const char *cached_rc) // start from empty now that nobody else can be writing it. if (ftruncate(fd, 0) < 0 || !(f = fdopen(fd, "w"))) { err = errno; - fprintf(stderr, "[cgit] Error writing %s: %s (%d)\n", locked_rc.buf, strerror(err), err); + fprintf(stderr, "[cgit] Unable to write %s: %s (%d)\n", locked_rc.buf, strerror(err), err); unlink(locked_rc.buf); close(fd); goto out; @@ -406,14 +413,14 @@ static int generate_cached_repolist(const char *path, const char *cached_rc) // that stops wherever the buffer happened to end. if (fflush(f) || ferror(f)) { err = errno; - fprintf(stderr, "[cgit] Error writing %s: %s (%d)\n", locked_rc.buf, strerror(err), err); + fprintf(stderr, "[cgit] Unable to write %s: %s (%d)\n", locked_rc.buf, strerror(err), err); unlink(locked_rc.buf); fclose(f); goto out; } if (rename(locked_rc.buf, cached_rc)) { err = errno; - fprintf(stderr, "[cgit] Error renaming %s to %s: %s (%d)\n", + fprintf(stderr, "[cgit] Unable to rename %s to %s: %s (%d)\n", locked_rc.buf, cached_rc, strerror(err), err); unlink(locked_rc.buf); } @@ -425,7 +432,9 @@ out: return err; } -// A cached repolist is itself a config file, so these two call each other. +/* + * A cached repolist is itself a config file, so these two call each other. + */ static void apply_config(const char *name, const char *value); static void process_cached_repolist(const char *path) @@ -539,8 +548,8 @@ static void apply_config(const char *name, const char *value) ctx.cfg.enable_header = atoi(value); else if (!strcmp(name, "snapshots")) ctx.cfg.snapshots = cgit_parse_snapshots_mask(value); - else if (!strcmp(name, "trust-scan-filters")) - ctx.cfg.trust_scan_filters = atoi(value); + else if (!strcmp(name, "trust-scan-config")) + ctx.cfg.trust_scan_config = atoi(value); else if (!strcmp(name, "enable-follow-links")) ctx.cfg.enable_follow_links = atoi(value); else if (!strcmp(name, "enable-http-clone")) @@ -644,6 +653,9 @@ static void apply_config(const char *name, const char *value) scan_projects(cgit_expand_macros(value), ctx.cfg.project_list); else scan_tree(cgit_expand_macros(value)); + // The scan grows the repository array, so a record taken before + // it may have moved, and a repo key after it belongs to nothing. + ctx.repo = NULL; } else if (!strcmp(name, "scan-hidden-path")) ctx.cfg.scan_hidden_path = atoi(value); else if (!strcmp(name, "section-from-path")) @@ -721,7 +733,7 @@ static void apply_query_param(const char *name, const char *value) if (!value) value = ""; - if (!strcmp(name,"r")) { + if (!strcmp(name, "r")) { ctx.qry.repo = xstrdup(value); ctx.repo = cgit_get_repoinfo(value); } else if (!strcmp(name, "p")) { @@ -814,10 +826,12 @@ static void authenticate_post(void) len = ctx.env.content_length; if (len > MAX_AUTHENTICATION_POST_BYTES) len = MAX_AUTHENTICATION_POST_BYTES; - if ((got = read(STDIN_FILENO, buffer, len)) < 0) - die_errno("Could not read POST from stdin"); - if (write(STDOUT_FILENO, buffer, got) < 0) - die_errno("Could not write POST to stdout"); + // The body can arrive in more than one write, so read until it is + // all there or the server closes the stream. + if ((got = read_in_full(STDIN_FILENO, buffer, len)) < 0) + die_errno("Unable to read the POST body"); + if (write_in_full(STDOUT_FILENO, buffer, got) < 0) + die_errno("Unable to pass the POST body to the auth filter"); cgit_close_filter(ctx.cfg.auth_filter); exit(0); } @@ -856,13 +870,42 @@ static int is_full_oid(const char *rev) if (len != GIT_SHA1_HEXSZ && len != GIT_SHA256_HEXSZ) return 0; for (; *rev; rev++) { - if (!isxdigit(*rev)) + if (!isxdigit((unsigned char)*rev)) return 0; } return 1; } -// Every cache-*-ttl setting is written in minutes, and so is this. +/* + * Only these pages render the same bytes for the same id for ever. A log or + * refs page named with an id still lists branches and tags, which move. + */ +static int page_is_static(void) +{ + static const char *const pages[] = { + "blame", + "blob", + "commit", + "diff", + "patch", + "plain", + "rawdiff", + "snapshot", + "tag", + "tree", + }; + size_t i; + + for (i = 0; i < ARRAY_SIZE(pages); i++) { + if (!strcmp(ctx.qry.page, pages[i])) + return 1; + } + return 0; +} + +/* + * Every cache-*-ttl setting is written in minutes, and so is this. + */ static int calc_ttl(void) { if (!ctx.repo) @@ -879,6 +922,7 @@ static int calc_ttl(void) // page to rebuild. if ( ctx.qry.has_oid && + page_is_static() && (!ctx.qry.oid || is_full_oid(ctx.qry.oid)) && (!ctx.qry.oid2 || is_full_oid(ctx.qry.oid2)) ) @@ -917,14 +961,18 @@ static void build_cache_key(struct strbuf *key) free(hosturl); } -// Returning non-zero ends git's walk, so the search stops at the first hit. +/* + * Returning non-zero ends git's walk, so the search stops at the first hit. + */ static int find_current_ref(const struct reference *ref, void *data) { struct refmatch *match = data; if (!strcmp(ref->name, match->wanted)) match->found = 1; - if (!match->first) + // The fallback has to pass the check every request makes on its head, + // or one branch named with a leading dash takes the repository offline. + if (!match->first && ref->name[0] != '-') match->first = xstrdup(ref->name); return match->found; } @@ -996,14 +1044,18 @@ static void parse_readme(const char *readme, char **filename, char **ref, struct static void choose_readme(struct cgit_repo *repo) { int found; - char *filename, *ref; + char *filename = NULL, *ref = NULL; + struct string_list *list; struct string_list_item *entry; - if (!repo->readme.nr) + // A repository without readme settings of its own follows the global + // ones, which it must not free. + list = repo->readme.nr ? &repo->readme : &ctx.cfg.readme; + if (!list->nr) return; found = 0; - for_each_string_list_item(entry, &repo->readme) { + for_each_string_list_item(entry, list) { parse_readme(entry->string, &filename, &ref, repo); if (!filename) { free(ref); @@ -1028,11 +1080,19 @@ static void choose_readme(struct cgit_repo *repo) string_list_append(&repo->readme, filename)->util = ref; } -static void prepare_repo_env(int *nongit) +static void prepare_repo_env(int *nongit, int *err) { + // A repository configured without a path has nothing to open. + if (!ctx.repo->path) { + *nongit = 1; + *err = 0; + return; + } setenv("GIT_DIR", ctx.repo->path, 1); + errno = 0; setup_git_directory_gently(the_repository, nongit); + *err = errno; // Notes come out of the object store, which a repository that failed // to open has none of. if (!*nongit) @@ -1043,14 +1103,12 @@ static void prepare_repo_env(int *nongit) * Returns non-zero once it has written a complete response of its own, in * which case the caller must not render a page over the top of it. */ -static int prepare_repo_cmd(int nongit) +static int prepare_repo_cmd(int nongit, int err, int clone) { struct object_id oid; - int err; if (nongit) { const char *name = ctx.repo->name; - err = errno; ctx.page.title = cgit_fmtalloc("%s - %s", ctx.cfg.root_title, "config error"); ctx.repo = NULL; cgit_print_error_page( @@ -1070,6 +1128,12 @@ static int prepare_repo_cmd(int nongit) } if (!ctx.qry.head) { + // The dumb transport serves refs and objects off the disk and + // asks nothing of the head, and an empty repository clones. + if (clone) { + cgit_prepare_repo_env(ctx.repo); + return 0; + } ctx.empty_repo = 1; // Before the document starts, since the <head> carries // <link rel='vcs-git'> and those clone urls expand macros such @@ -1083,13 +1147,22 @@ static int prepare_repo_cmd(int nongit) return 1; } - if (repo_get_oid(the_repository, ctx.qry.head, &oid)) { + // Every revision the request names is checked before git sees it, + // see cgit_valid_rev, and the head has to resolve as well. + if (!cgit_valid_rev(ctx.qry.head) || repo_get_oid(the_repository, ctx.qry.head, &oid)) { char *old_head = ctx.qry.head; ctx.qry.head = xstrdup(ctx.repo->defbranch); cgit_print_error_page(404, "Not Found", "Invalid branch: %s", old_head); free(old_head); return 1; } + if ( + (ctx.qry.oid && !cgit_valid_rev(ctx.qry.oid)) || + (ctx.qry.oid2 && !cgit_valid_rev(ctx.qry.oid2)) + ) { + cgit_print_error_page(400, "Bad Request", "Invalid revision"); + return 1; + } string_list_sort(&ctx.repo->submodules); cgit_prepare_repo_env(ctx.repo); choose_readme(ctx.repo); @@ -1099,7 +1172,7 @@ static int prepare_repo_cmd(int nongit) static void process_request(void) { const struct cgit_cmd *cmd; - int nongit = 0; + int nongit = 0, err = 0; // An unauthenticated request is answered with the filter's own body // whatever page it asked for. @@ -1115,16 +1188,14 @@ static void process_request(void) } if (ctx.repo) - prepare_repo_env(&nongit); + prepare_repo_env(&nongit, &err); cmd = cgit_get_cmd(); - if (!cmd) { - ctx.page.title = "cgit error"; - cgit_print_error_page(404, "Not Found", "Invalid request"); - return; - } - - if (!ctx.cfg.enable_http_clone && cmd->is_clone) { + if (!cmd || (!ctx.cfg.enable_http_clone && cmd->is_clone)) { + // The error page carries the repository's own header, whose + // branch switcher needs the head resolved first. + if (ctx.repo && prepare_repo_cmd(nongit, err, 0)) + return; ctx.page.title = "cgit error"; cgit_print_error_page(404, "Not Found", "Invalid request"); return; @@ -1137,7 +1208,7 @@ static void process_request(void) ctx.qry.vpath = cmd->want_vpath ? ctx.qry.path : NULL; - if (ctx.repo && prepare_repo_cmd(nongit)) + if (ctx.repo && prepare_repo_cmd(nongit, err, cmd->is_clone)) return; cmd->fn(); @@ -1205,13 +1276,11 @@ void cgit_repo_config(struct cgit_repo *repo, const char *name, const char *valu repo->section = cgit_strdup_first_line(value); else if (!strcmp(name, "snapshot-prefix")) repo->snapshot_prefix = cgit_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)); + else if (!strcmp(name, "readme")) string_list_append(&repo->readme, cgit_strdup_first_line(value)); - } else if (!strcmp(name, "logo") && value != NULL) + else if (!strcmp(name, "logo")) repo->logo = cgit_strdup_first_line(value); - else if (!strcmp(name, "logo-link") && value != NULL) + else if (!strcmp(name, "logo-link")) repo->logo_link = cgit_strdup_first_line(value); else if (!strcmp(name, "hide")) repo->hide = atoi(value); @@ -1238,6 +1307,9 @@ int cmd_main(int argc, const char **argv) int err, ttl; isolate_git_environment(); + // Ignored, a filter that exits early makes the write fail with EPIPE + // and the error page reach the visitor, instead of ending cgit. + signal(SIGPIPE, SIG_IGN); cgit_init_filters(); atexit(cgit_cleanup_filters); // Registered second so it runs first, since exit is reached from error @@ -1271,8 +1343,9 @@ int cmd_main(int argc, const char **argv) char *path_and_query = cgit_fmtalloc("%s?%s", path, ctx.qry.raw); free(ctx.qry.raw); ctx.qry.raw = path_and_query; - } else + } else { ctx.qry.raw = xstrdup(ctx.qry.url); + } cgit_parse_url(ctx.qry.url); } diff --git a/source/cgit.h b/source/cgit.h index 72c0844..163cd4d 100644 --- a/source/cgit.h +++ b/source/cgit.h @@ -38,6 +38,7 @@ #include <string-list.h> #include <strvec.h> #include <tag.h> +#include <tree-walk.h> #include <tree.h> #include <url.h> #include <utf8.h> @@ -58,6 +59,13 @@ // A double, because a twelfth of a year is not a whole number of seconds. #define SECONDS_PER_MONTH (SECONDS_PER_YEAR / 12.0) +// The number of context lines git itself defaults to. +#define DEFAULT_DIFF_CONTEXT 3 + +// A tab in rendered source runs on to the next multiple of this, which is the +// width a browser picks on its own since the stylesheet sets no tab-size. +#define TAB_WIDTH 8 + typedef enum { DIFF_UNIFIED, DIFF_SSDIFF, DIFF_STATONLY } diff_type; @@ -168,7 +176,7 @@ struct cgit_query { char *path; char *url; char *period; - int ofs; + int ofs; int nohead; char *sort; int showmsg; @@ -203,7 +211,8 @@ struct cgit_config { char *script_name; char *section; char *repository_sort; - char *virtual_root; // Always ends with a slash. + // Always ends with a slash. + char *virtual_root; char *strict_export; struct date_mode date_mode; int cache_size; @@ -216,7 +225,7 @@ struct cgit_config { int cache_snapshot_ttl; int case_sensitive_sort; int embedded; - int trust_scan_filters; + int trust_scan_config; int enable_follow_links; int enable_gitmodules_links; int enable_header; @@ -281,6 +290,8 @@ struct cgit_page { int status; const char *statusmsg; int untrusted; + // Set once the status line is out, when an error can only join the body. + int headers_sent; }; struct cgit_environment { diff --git a/source/cgit.mk b/source/cgit.mk index 1199278..4109930 100644 --- a/source/cgit.mk +++ b/source/cgit.mk @@ -27,18 +27,14 @@ CGIT_BUILD = $(CGIT_ROOT)/$(BUILDDIR) # The CGIT_ values used below are exported by the top level Makefile. $(CGIT_BUILD)/VERSION: force-version @mkdir -p $(CGIT_BUILD)/ - @cd $(CGIT_ROOT) && '$(SHELL_PATH_SQ)' $(TOOLSDIR)/gen-version.sh "$(CGIT_VERSION)" $(BUILDDIR)/VERSION + @cd $(CGIT_ROOT) && \ + '$(SHELL_PATH_SQ)' $(TOOLSDIR)/gen-version.sh "$(CGIT_VERSION)" $(BUILDDIR)/VERSION -include $(CGIT_BUILD)/VERSION .PHONY: force-version -# The language the cgit sources are written in. Both GCC and Clang default to -# this today, so pinning it changes nothing now and stops the meaning of the -# sources drifting when a compiler moves its default on, as GCC 15 did by -# defaulting to gnu23. The GNU dialect rather than plain c17 because git's -# headers use GNU extensions, and because dlsym cannot be used through a -# conforming cast. Only the cgit objects are held to this, and Git keeps -# whatever its own build decides, which on some platforms is a different -# standard again. +# Pinned so a compiler moving its default, as GCC 15 did to gnu23, cannot +# change what the sources mean. The GNU dialect because git's headers use GNU +# extensions and dlsym needs a non-conforming cast. Git keeps its own choice. CGIT_STD ?= gnu17 # CGIT_CFLAGS is tracked separately so that changing it does not force a rebuild @@ -157,12 +153,14 @@ $(CGIT_BUILD)/.depend: $(CGIT_BUILD)/CGIT-CFLAGS: FORCE @mkdir -p $(CGIT_BUILD)/ @FLAGS='$(subst ','\'',$(CGIT_CFLAGS))'; \ - if test x"$$FLAGS" != x"`cat $(CGIT_BUILD)/CGIT-CFLAGS 2>/dev/null`" ; then \ - echo 1>&2 " * new CGit build flags"; \ - echo "$$FLAGS" >$(CGIT_BUILD)/CGIT-CFLAGS; \ - fi - -$(CGIT_OBJS): $(CGIT_BUILD)/%.o: $(CGIT_SRC)/%.c GIT-CFLAGS $(CGIT_BUILD)/CGIT-CFLAGS $(missing_dep_dirs) + OLD=`cat $(CGIT_BUILD)/CGIT-CFLAGS 2>/dev/null`; \ + if test x"$$FLAGS" != x"$$OLD"; then \ + echo 1>&2 " * new CGit build flags"; \ + echo "$$FLAGS" >$(CGIT_BUILD)/CGIT-CFLAGS; \ + fi + +$(CGIT_OBJS): $(CGIT_BUILD)/%.o: $(CGIT_SRC)/%.c GIT-CFLAGS $(CGIT_BUILD)/CGIT-CFLAGS \ + $(missing_dep_dirs) $(QUIET_CC)$(CC) -o $@ -c $(dep_args) $(ALL_CFLAGS) $(EXTRA_CPPFLAGS) $(CGIT_CFLAGS) $< $(CGIT_BUILD)/cgit: $(CGIT_OBJS) GIT-LDFLAGS $(GITLIBS) diff --git a/source/cmd.c b/source/cmd.c index 2c6406f..a19aa50 100644 --- a/source/cmd.c +++ b/source/cmd.c @@ -212,7 +212,7 @@ const struct cgit_cmd *cgit_get_cmd(void) { size_t i; - if (ctx.qry.page == NULL) { + if (!ctx.qry.page) { if (ctx.repo) ctx.qry.page = "summary"; else diff --git a/source/cmd.h b/source/cmd.h index a75cdd0..f580fa1 100644 --- a/source/cmd.h +++ b/source/cmd.h @@ -16,7 +16,9 @@ struct cgit_cmd { unsigned int want_repo:1, want_vpath:1, is_clone:1; }; -// The entry naming ctx.qry.page, or NULL when no page goes by that name. +/* + * The entry naming ctx.qry.page, or NULL when no page goes by that name. + */ extern const struct cgit_cmd *cgit_get_cmd(void); #endif // CGIT_CMD_H diff --git a/source/config.c b/source/config.c index 0ff8588..2e79791 100644 --- a/source/config.c +++ b/source/config.c @@ -70,7 +70,7 @@ static int read_entry(FILE *f, struct strbuf *name, struct strbuf *value) } if (c == EOF) return 0; - // Dropping just the line with no equals sign keeps one typo + // Dropping only the line with no equals sign keeps one typo // from hiding the rest of the file. if (c != '=') continue; diff --git a/source/filter.c b/source/filter.c index 1cfdde2..f5fb2d5 100644 --- a/source/filter.c +++ b/source/filter.c @@ -22,6 +22,9 @@ // for a command not found. #define EXEC_FAILED 127 +// The exec filter holding stdout, for cgit_abort_filters to take it back. +static struct cgit_exec_filter *running_exec; + static int open_exec_filter(struct cgit_filter *base, va_list ap) { struct cgit_exec_filter *filter = (struct cgit_exec_filter *)base; @@ -31,13 +34,19 @@ static int open_exec_filter(struct cgit_filter *base, va_list ap) for (i = 0; i < filter->base.argument_count; i++) filter->argv[i + 1] = va_arg(ap, char *); - filter->old_stdout = cgit_die_unless_positive(dup(STDOUT_FILENO), "Unable to duplicate STDOUT"); + filter->old_stdout = cgit_die_unless_positive(dup(STDOUT_FILENO), "Unable to duplicate stdout"); + // The child keeps the page's stdout, but not the descriptor the parent + // needs it back from. + fcntl(filter->old_stdout, F_SETFD, FD_CLOEXEC); cgit_die_unless_zero(pipe(pipefd), "Unable to create pipe to subprocess"); filter->pid = cgit_die_unless_non_negative(fork(), "Unable to create subprocess"); if (filter->pid == 0) { close(pipefd[1]); if (dup2(pipefd[0], STDIN_FILENO) < 0) _exit(EXEC_FAILED); + // cgit ignores SIGPIPE and the disposition survives exec, so a + // filter that writes to a closed pipe would otherwise carry on. + signal(SIGPIPE, SIG_DFL); execvp(filter->cmd, filter->argv); // The child shares the parent's page buffer, cache lock and exit // handlers, so it must not die through them. @@ -45,11 +54,9 @@ static int open_exec_filter(struct cgit_filter *base, va_list ap) _exit(EXEC_FAILED); } close(pipefd[0]); - // The child keeps the page's stdout, but not the descriptor the parent - // needs it back from. - fcntl(filter->old_stdout, F_SETFD, FD_CLOEXEC); - cgit_die_unless_non_negative(dup2(pipefd[1], STDOUT_FILENO), "Unable to use pipe as STDOUT"); + cgit_die_unless_non_negative(dup2(pipefd[1], STDOUT_FILENO), "Unable to use pipe as stdout"); close(pipefd[1]); + running_exec = filter; return 0; } @@ -58,7 +65,8 @@ static int close_exec_filter(struct cgit_filter *base) struct cgit_exec_filter *filter = (struct cgit_exec_filter *)base; int i, exit_status = 0; - cgit_die_unless_non_negative(dup2(filter->old_stdout, STDOUT_FILENO), "Unable to restore STDOUT"); + running_exec = NULL; + cgit_die_unless_non_negative(dup2(filter->old_stdout, STDOUT_FILENO), "Unable to restore stdout"); close(filter->old_stdout); if (filter->pid < 0) goto done; @@ -77,12 +85,14 @@ done: static void fprintf_exec_filter(struct cgit_filter *base, FILE *f, const char *prefix) { struct cgit_exec_filter *filter = (struct cgit_exec_filter *)base; + fprintf(f, "%sexec:%s\n", prefix, filter->cmd); } static void cleanup_exec_filter(struct cgit_filter *base) { struct cgit_exec_filter *filter = (struct cgit_exec_filter *)base; + free(filter->argv); filter->argv = NULL; free(filter->cmd); @@ -141,7 +151,7 @@ void cgit_init_filters(void) { libc_write = dlsym(RTLD_NEXT, "write"); if (!libc_write) - die("Could not locate libc's write function"); + die("Unable to find libc's write function"); } /* @@ -159,16 +169,16 @@ static inline void hook_write(struct cgit_filter *filter, filter_write_fn write_ { // Filters cannot nest, because there is one stdout and one hook, so a // second one would strand the first. - assert(filter_write == NULL); - assert(current_write_filter == NULL); + assert(!filter_write); + assert(!current_write_filter); current_write_filter = filter; filter_write = write_fn; } static inline void unhook_write(void) { - assert(filter_write != NULL); - assert(current_write_filter != NULL); + assert(filter_write); + assert(current_write_filter); filter_write = NULL; current_write_filter = NULL; } @@ -322,7 +332,7 @@ static int close_lua_filter(struct cgit_filter *base) lua_getglobal(filter->lua_state, "filter_close"); if (lua_pcall(filter->lua_state, 0, 1, 0)) die_lua_error(filter); - ret = lua_tonumber(filter->lua_state, -1); + ret = (int)lua_tointeger(filter->lua_state, -1); lua_pop(filter->lua_state, 1); unhook_write(); @@ -368,6 +378,7 @@ int cgit_open_filter(struct cgit_filter *filter, ...) { int result; va_list ap; + if (!filter) return 0; // Whatever is still buffered belongs to the page, not to the filter @@ -403,6 +414,7 @@ static inline void cleanup_filter(struct cgit_filter *filter) void cgit_cleanup_filters(void) { int i; + cleanup_filter(ctx.cfg.about_filter); cleanup_filter(ctx.cfg.commit_filter); cleanup_filter(ctx.cfg.source_filter); @@ -418,6 +430,24 @@ void cgit_cleanup_filters(void) } } +/* + * Take stdout back from a filter that holds it when a die lands, so the error + * page reaches the visitor and not the filter. What the page had buffered for + * the filter is dropped with it, since it was never meant to go out as it is. + * An exec child sees the end of its input and exits on its own. + */ +void cgit_abort_filters(void) +{ + html_discard(); + if (running_exec) { + dup2(running_exec->old_stdout, STDOUT_FILENO); + close(running_exec->old_stdout); + running_exec = NULL; + } + if (filter_write) + unhook_write(); +} + static const struct { const char *prefix; struct cgit_filter *(*create)(const char *cmd, int argument_count); @@ -473,5 +503,5 @@ struct cgit_filter *cgit_new_filter(const char *cmd, filter_type filtertype) return filter_specs[i].create(colon + 1, argument_count); } - die("Invalid filter type: %.*s", (int) len, cmd); + die("Invalid filter type: %.*s", (int)len, cmd); } diff --git a/source/filter.h b/source/filter.h index fa43416..b5a478f 100644 --- a/source/filter.h +++ b/source/filter.h @@ -44,11 +44,19 @@ extern void cgit_exec_filter_init(struct cgit_exec_filter *filter, char *cmd, extern int cgit_open_filter(struct cgit_filter *filter, ...); extern int cgit_close_filter(struct cgit_filter *filter); -// Describe the filter the way cgitrc would have configured it. +/* + * Describe the filter the way cgitrc would have configured it. + */ extern void cgit_fprintf_filter(struct cgit_filter *filter, FILE *f, const char *prefix); extern void cgit_init_filters(void); extern void cgit_cleanup_filters(void); +/* + * Take stdout back from whichever filter holds it, dropping what was buffered + * for that filter. For the die path only. + */ +extern void cgit_abort_filters(void); + #endif // CGIT_FILTER_H diff --git a/source/html.c b/source/html.c index 5972d97..a1d244b 100644 --- a/source/html.c +++ b/source/html.c @@ -9,6 +9,7 @@ */ #include "cgit.h" +#include "cache.h" #include "html.h" #define HTML_WRITE_BUFSIZE (64 * 1024) @@ -58,8 +59,22 @@ static void write_out(const char *data, size_t size) { // A blob or snapshot is well past what one write can move onto a // pipe, so short writes are resumed rather than reported. - if (write_in_full(STDOUT_FILENO, data, size) < 0) - die_errno("write error on html output"); + while (size) { + ssize_t written = xwrite(STDOUT_FILENO, data, size); + + if (written < 0) + break; + data += written; + size -= written; + } + if (!size) + return; + // While a cache slot is being filled stdout is a file, and a full disk + // must not cost the visitor the page. Abandoning the fill puts stdout + // back on the client along with what the file already holds, so only + // the rest is written again. + if (!cache_abandon_fill() || write_in_full(STDOUT_FILENO, data, size) < 0) + die_errno("Unable to write the page"); } /* @@ -111,6 +126,11 @@ void html_flush(void) write_out(out_buf, len); } +void html_discard(void) +{ + out_len = 0; +} + void html_capture_begin(struct strbuf *sb) { html_flush(); @@ -191,7 +211,7 @@ ssize_t html_ntxt(const char *txt, size_t len) if (len > SSIZE_MAX) return -1; - left = (ssize_t) len; + left = (ssize_t)len; while (p && *p && left--) { int c = *p; if (c == '<' || c == '>' || c == '&') { @@ -227,6 +247,7 @@ void html_attrf(const char *format, ...) void html_attr(const char *txt) { const char *p = txt; + while (p && *p) { int c = *p; if (c == '<' || c == '>' || c == '\'' || c == '\"' || c == '&') { @@ -252,6 +273,7 @@ void html_attr(const char *txt) void html_url_path(const char *txt) { const char *p = txt; + while (p && *p) { unsigned char c = *p; // A raw ampersand or plus is legal in a URL path, but this @@ -274,6 +296,7 @@ void html_url_path(const char *txt) void html_url_arg(const char *txt) { const char *p = txt; + while (p && *p) { unsigned char c = *p; const char *esc = url_escape_table[c]; @@ -293,6 +316,7 @@ void html_url_arg(const char *txt) void html_header_arg_in_quotes(const char *txt) { const char *p = txt; + while (p && *p) { unsigned char c = *p; const char *esc = NULL; @@ -379,7 +403,7 @@ int html_include(const char *filename) size_t len; if (!(f = fopen(filename, "r"))) { - fprintf(stderr, "[cgit] Error including file %s: %s (%d)\n", filename, strerror(errno), errno); + fprintf(stderr, "[cgit] Unable to include %s: %s (%d)\n", filename, strerror(errno), errno); return -1; } while ((len = fread(buf, 1, sizeof(buf), f)) > 0) diff --git a/source/html.h b/source/html.h index 410661b..46b445f 100644 --- a/source/html.h +++ b/source/html.h @@ -12,10 +12,9 @@ #include "cgit.h" -// How much of a generated run to build before handing it to html_raw. Growing -// past this gains nothing, since html_raw gathers what it is given into a -// buffer of its own, and a run built whole would grow with the file it came -// from, to many times the blob's size, which max-blob-size never measures. +// How much of a generated run to build before handing it to html_raw. A run +// built whole would grow with the file it came from, to many times the blob's +// size, which max-blob-size never measures. #define HTML_BATCH (64 * 1024) extern void html_raw(const char *txt, size_t size); @@ -28,6 +27,11 @@ extern void html(const char *txt); extern void html_flush(void); /* + * Drop whatever page output is still buffered. For the die path only. + */ +extern void html_discard(void); + +/* * Collect page output into sb rather than writing it, until html_capture_end. * A capture must not span a filter or anything else that writes to stdout * directly, since that output would escape the capture. diff --git a/source/parsing.c b/source/parsing.c index 5fde25d..655973b 100644 --- a/source/parsing.c +++ b/source/parsing.c @@ -31,11 +31,9 @@ static char *substr(const char *start, const char *end) return buf; } -/* - * The repository's mailmap, read once per request and applied to every ident - * a page shows. Only the blob at HEAD is read, the way git reads a bare - * repository, so a repository cannot point the reader at a file on the server. - */ +// The repository's mailmap, read once per request and applied to every ident a +// page shows. Only the blob at HEAD is read, the way git reads a bare +// repository, so a repository cannot point the reader at a file on the server. static struct string_list mailmap = STRING_LIST_INIT_DUP; static int mailmap_read; @@ -175,7 +173,7 @@ struct commitinfo *cgit_parse_commit(struct commit *commit) return info; if (!skip_prefix(p, "tree ", &p)) - die("Bad commit: %s", oid_to_hex(&commit->object.oid)); + die("Unable to parse commit %s", oid_to_hex(&commit->object.oid)); p += the_hash_algo->hexsz + 1; while (skip_prefix(p, "parent ", &p)) @@ -187,7 +185,8 @@ struct commitinfo *cgit_parse_commit(struct commit *commit) } if (p && skip_prefix(p, "committer ", &p)) { - parse_user(p, &info->committer, &info->committer_email, &info->committer_date, &info->committer_tz); + parse_user(p, &info->committer, &info->committer_email, &info->committer_date, + &info->committer_tz); p = next_header_line(p); } @@ -247,9 +246,8 @@ struct taginfo *cgit_parse_tag(struct tag *tag) info = xcalloc(1, sizeof(struct taginfo)); for (p = data; !end_of_header(p); p = next_header_line(p)) { - if (skip_prefix(p, "tagger ", &p)) { + if (skip_prefix(p, "tagger ", &p)) parse_user(p, &info->tagger, &info->tagger_email, &info->tagger_date, &info->tagger_tz); - } } while (p && *p == '\n') diff --git a/source/parsing.h b/source/parsing.h index d213429..755d646 100644 --- a/source/parsing.h +++ b/source/parsing.h @@ -10,8 +10,10 @@ #include "cgit.h" -// Split a request url into the repository, the page and the path it names, -// storing them in ctx.qry. +/* + * Split a request url into the repository, the page and the path it names, + * storing them in ctx.qry. + */ extern void cgit_parse_url(const char *url); extern struct string_list *cgit_mailmap(void); diff --git a/source/scan-tree.c b/source/scan-tree.c index ba2149a..4110511 100644 --- a/source/scan-tree.c +++ b/source/scan-tree.c @@ -32,7 +32,7 @@ static int stat_entry(const char *dir, const char *name, struct stat *st) // A missing entry is the ordinary answer for a directory that is not a // repository, so only some other failure is worth reporting. if (err && errno != ENOENT) - fprintf(stderr, "[cgit] Error checking path %s: %s (%d)\n", dir, strerror(errno), errno); + fprintf(stderr, "[cgit] Unable to stat %s: %s (%d)\n", dir, strerror(errno), errno); strbuf_release(&path); return err; } @@ -48,13 +48,30 @@ static int is_git_dir(const char *path) return 1; } -// A filter is a command cgit runs, and a repository's own files belong to -// whoever can push to it, so their filter keys wait on trust-scan-filters. -static int trusted_key(const char *name) +/* + * A repository's own files belong to whoever can push to it, so the keys + * that run a command, put raw markup on the page, read a file off the disk + * or place a link wait on trust-scan-config. A readme naming a git object, + * the form with a colon, only reads from the repository and passes. + */ +static int trusted_key(const char *name, const char *value) { - if (!ends_with(name, "-filter") || ctx.cfg.trust_scan_filters) + int untrusted; + + if (ctx.cfg.trust_scan_config) return 1; - fprintf(stderr, "[cgit] Ignoring %s in %s: trust-scan-filters is not set\n", name, + untrusted = + ends_with(name, "-filter") || + !strcmp(name, "head-content") || + !strcmp(name, "module-link") || + starts_with(name, "module-link.") || + !strcmp(name, "logo") || + !strcmp(name, "logo-link") || + !strcmp(name, "clone-url") || + (!strcmp(name, "readme") && !strchr(value, ':')); + if (!untrusted) + return 1; + fprintf(stderr, "[cgit] Ignoring %s in %s: trust-scan-config is not set\n", name, current_repo->path); return 0; } @@ -64,13 +81,16 @@ static int apply_gitconfig(const char *key, const char *value, { const char *name; + // A key with no value is legal git config and nothing here can use it. + if (!value) + return 0; if (!strcmp(key, "gitweb.owner")) cgit_repo_config(current_repo, "owner", value); else if (!strcmp(key, "gitweb.description")) cgit_repo_config(current_repo, "desc", value); else if (!strcmp(key, "gitweb.category")) cgit_repo_config(current_repo, "section", value); - else if (skip_prefix(key, "cgit.", &name) && trusted_key(name)) + else if (skip_prefix(key, "cgit.", &name) && trusted_key(name, value)) cgit_repo_config(current_repo, name, value); return 0; @@ -78,7 +98,7 @@ static int apply_gitconfig(const char *key, const char *value, static void apply_cgitrc(const char *name, const char *value) { - if (trusted_key(name)) + if (trusted_key(name, value)) cgit_repo_config(current_repo, name, value); } @@ -135,7 +155,7 @@ static void add_repo(const char *base, struct strbuf *path) size_t desc_size; if (stat(path->buf, &st)) { - fprintf(stderr, "[cgit] Error accessing %s: %s (%d)\n", path->buf, strerror(errno), errno); + fprintf(stderr, "[cgit] Unable to access %s: %s (%d)\n", path->buf, strerror(errno), errno); return; } @@ -143,8 +163,10 @@ static void add_repo(const char *base, struct strbuf *path) pathlen = path->len; if (ctx.cfg.strict_export) { + struct stat export_st; + strbuf_addstr(path, ctx.cfg.strict_export); - if (stat(path->buf, &st)) + if (stat(path->buf, &export_st)) return; strbuf_setlen(path, pathlen); } @@ -165,11 +187,31 @@ static void add_repo(const char *base, struct strbuf *path) strbuf_setlen(&relpath, relpath.len - 1); if (relpath.len >= 5 && !strcmp(relpath.buf + relpath.len - 5, "/.git")) strbuf_setlen(&relpath, relpath.len - 5); + else if (!strcmp(relpath.buf, ".git")) + strbuf_setlen(&relpath, 0); + // The scan root may itself be the repository, leaving nothing after + // the base, so that one is named after its directory. + if (!relpath.len) { + const char *end = path->buf + pathlen - 1, *start; + + if (end - path->buf >= 5 && !strncmp(end - 5, "/.git", 5)) + end -= 5; + start = end; + while (start > path->buf && start[-1] != '/') + start--; + strbuf_add(&relpath, start, end - start); + } current_repo = cgit_add_repo(relpath.buf); if (ctx.cfg.enable_git_config) { + // A pusher wrote this file, so a broken one is skipped with a + // warning rather than allowed to end the request. + struct config_options opts = { .error_action = CONFIG_ERROR_SILENT }; + strbuf_addstr(path, "config"); - git_config_from_file(apply_gitconfig, path->buf, NULL); + if (git_config_from_file_with_options(apply_gitconfig, path->buf, NULL, + CONFIG_SCOPE_UNKNOWN, &opts)) + fprintf(stderr, "[cgit] Ignoring unreadable config in %s\n", path->buf); strbuf_setlen(path, pathlen); } @@ -181,16 +223,15 @@ static void add_repo(const char *base, struct strbuf *path) } current_repo->path = cgit_strdup_first_line(path->buf); while (!current_repo->owner) { - if ((pwd = getpwuid(st.st_uid)) == NULL) { - fprintf(stderr, "[cgit] Error reading owner-info for %s: %s (%d)\n", + if (!(pwd = getpwuid(st.st_uid))) { + fprintf(stderr, "[cgit] Unable to read the owner of %s: %s (%d)\n", path->buf, strerror(errno), errno); break; } // A gecos field puts the owner's name in front of a comma // separated list of office and phone details. - if (pwd->pw_gecos) - if ((comma = strchr(pwd->pw_gecos, ','))) - *comma = '\0'; + if (pwd->pw_gecos && (comma = strchr(pwd->pw_gecos, ','))) + *comma = '\0'; current_repo->owner = cgit_strdup_first_line(pwd->pw_gecos ? pwd->pw_gecos : pwd->pw_name); } @@ -210,7 +251,7 @@ static void add_repo(const char *base, struct strbuf *path) strbuf_addstr(path, "cgitrc"); if (!stat(path->buf, &st)) - config_file_parse(path->buf, &apply_cgitrc); + config_file_parse(path->buf, apply_cgitrc); strbuf_release(&relpath); } @@ -226,16 +267,45 @@ static int should_scan(const struct dirent *ent) return ctx.cfg.scan_hidden_path; } +// Symlinks are followed so that a link into the scan path counts, which means +// a link back at an ancestor would recurse until the path ran out of room. +// Each directory is entered once. +static struct { + dev_t dev; + ino_t ino; +} *visited; +static size_t visited_nr, visited_alloc; + +static int already_visited(const char *path) +{ + struct stat st; + size_t i; + + if (stat(path, &st)) + return 0; + for (i = 0; i < visited_nr; i++) + if (visited[i].dev == st.st_dev && visited[i].ino == st.st_ino) + return 1; + ALLOC_GROW(visited, visited_nr + 1, visited_alloc); + visited[visited_nr].dev = st.st_dev; + visited[visited_nr].ino = st.st_ino; + visited_nr++; + return 0; +} + static void scan_path(const char *base, const char *path) { - DIR *dir = opendir(path); + DIR *dir; struct dirent *ent; struct strbuf pathbuf = STRBUF_INIT; size_t pathlen = strlen(path); struct stat st; + if (already_visited(path)) + return; + dir = opendir(path); if (!dir) { - fprintf(stderr, "[cgit] Error opening directory %s: %s (%d)\n", path, strerror(errno), errno); + fprintf(stderr, "[cgit] Unable to open %s: %s (%d)\n", path, strerror(errno), errno); return; } @@ -252,13 +322,14 @@ static void scan_path(const char *base, const char *path) // Take in the '/' that "/.git" left in the buffer, since the loop below // truncates to this length and then appends an entry name straight on. pathlen++; - while ((ent = readdir(dir)) != NULL) { + while ((ent = readdir(dir))) { if (!should_scan(ent)) continue; strbuf_setlen(&pathbuf, pathlen); strbuf_addstr(&pathbuf, ent->d_name); if (stat(pathbuf.buf, &st)) { - fprintf(stderr, "[cgit] Error checking path %s: %s (%d)\n", pathbuf.buf, strerror(errno), errno); + fprintf(stderr, "[cgit] Unable to stat %s: %s (%d)\n", pathbuf.buf, strerror(errno), + errno); continue; } if (S_ISDIR(st.st_mode)) @@ -277,7 +348,7 @@ void scan_projects(const char *path, const char *projectsfile) projects = fopen(projectsfile, "r"); if (!projects) { - fprintf(stderr, "[cgit] Error opening projectsfile %s: %s (%d)\n", + fprintf(stderr, "[cgit] Unable to open project list %s: %s (%d)\n", projectsfile, strerror(errno), errno); return; } @@ -289,7 +360,7 @@ void scan_projects(const char *path, const char *projectsfile) scan_path(path, line.buf); } if ((err = ferror(projects))) { - fprintf(stderr, "[cgit] Error reading from projectsfile %s: %s (%d)\n", + fprintf(stderr, "[cgit] Unable to read project list %s: %s (%d)\n", projectsfile, strerror(err), err); } fclose(projects); diff --git a/source/shared.c b/source/shared.c index 6ac2353..a1d1597 100644 --- a/source/shared.c +++ b/source/shared.c @@ -14,8 +14,8 @@ #define MACRO_EXPANSION_BUFSIZE (1024 * 8) -// The number of context lines git itself defaults to. -#define DEFAULT_DIFF_CONTEXT 3 +// How much of a file is read to find its first line. +#define MAX_FIRST_LINE_READ (1024 * 64) typedef struct { const char *name; @@ -68,9 +68,29 @@ static struct refinfo *make_refinfo(const char *refname, const struct object_id return ref; } +/* + * Whether a revision from the request may reach git. A hex object id or a name + * that could be a ref passes. Git's wider syntax is refused, since :/pattern + * and rev^{/pattern} walk the whole history for a match, and a leading dash + * reads as an option further down. + */ +int cgit_valid_rev(const char *rev) +{ + const char *p = rev; + + if (!rev || !*rev || *rev == '-') + return 0; + while (isxdigit((unsigned char)*p)) + p++; + if (!*p) + return 1; + return !check_refname_format(rev, REFNAME_ALLOW_ONELEVEL); +} + static int load_mmfile(mmfile_t *file, const struct object_id *oid) { enum object_type type; + size_t size; // A null oid is the absent side of an add or a delete, and diffs as an // empty file rather than as a failure. @@ -80,10 +100,11 @@ static int load_mmfile(mmfile_t *file, const struct object_id *oid) return 1; } - file->ptr = odb_read_object(the_repository->objects, oid, &type, (unsigned long *)&file->size); - // odb_read_object leaves size untouched when it fails, so the caller - // has to be told rather than handed a buffer with an unset length. - return file->ptr != NULL; + file->ptr = odb_read_object(the_repository->objects, oid, &type, &size); + if (!file->ptr) + return 0; + file->size = size; + return 1; } /* @@ -106,7 +127,7 @@ static int emit_line(void *priv, mmbuffer_t *mb, int nbuf) int i; for (i = 0; i < nbuf; i++) { - if (mb[i].ptr[mb[i].size-1] != '\n') { + if (mb[i].ptr[mb[i].size - 1] != '\n') { fragment = xrealloc(fragment, fragment_len + mb[i].size); memcpy(fragment + fragment_len, mb[i].ptr, mb[i].size); fragment_len += mb[i].size; @@ -133,8 +154,10 @@ static int emit_line(void *priv, mmbuffer_t *mb, int nbuf) return 0; } -// Takes an unsigned char because a byte over 0x7f is negative where char -// is signed, which the ctype tests are not defined for. +/* + * Takes an unsigned char because a byte over 0x7f is negative where char is + * signed, which the ctype tests are not defined for. + */ static int is_token_char(unsigned char c) { return isalnum(c) || c == '_'; @@ -192,14 +215,22 @@ struct cgit_repo *cgit_add_repo(const char *url) cgit_repolist.length = 8; else cgit_repolist.length *= 2; - cgit_repolist.repos = xrealloc(cgit_repolist.repos, cgit_repolist.length * sizeof(struct cgit_repo)); + cgit_repolist.repos = + xrealloc(cgit_repolist.repos, cgit_repolist.length * sizeof(struct cgit_repo)); } - repo = &cgit_repolist.repos[cgit_repolist.count-1]; + repo = &cgit_repolist.repos[cgit_repolist.count - 1]; memset(repo, 0, sizeof(struct cgit_repo)); repo->url = cgit_trim_end(url, '/'); - if (repo->url) + if (repo->url) { *strchrnul(repo->url, '\n') = '\0'; + } else { + // Nothing can address a repository with no url, and every lookup + // compares the url, so it is kept out of the way. + fprintf(stderr, "[cgit] Ignoring repository with an empty url\n"); + repo->url = xstrdup(""); + repo->ignore = 1; + } repo->name = repo->url; repo->path = NULL; repo->desc = cgit_default_repo_desc; @@ -222,7 +253,6 @@ struct cgit_repo *cgit_add_repo(const char *url) repo->branch_sort = ctx.cfg.branch_sort; repo->commit_sort = ctx.cfg.commit_sort; repo->module_link = ctx.cfg.module_link; - repo->readme = ctx.cfg.readme; repo->mtime = -1; repo->about_filter = ctx.cfg.about_filter; repo->commit_filter = ctx.cfg.commit_filter; @@ -266,7 +296,7 @@ char *cgit_trim_end(const char *str, char c) { size_t len; - if (str == NULL) + if (!str) return NULL; len = strlen(str); while (len > 0 && str[len - 1] == c) @@ -286,7 +316,7 @@ char *cgit_ensure_end(const char *str, char c) result = xmalloc(len + 2); memcpy(result, str, len); - result[len] = '/'; + result[len] = c; result[len + 1] = '\0'; return result; } @@ -367,9 +397,15 @@ int cgit_diff_files(const struct object_id *old_oid, const struct object_id *new // Read the object headers first so an oversized blob is never inflated // into memory just to be diffed. Reporting it as binary suppresses // inlining the same way max-blob-size does in the other views. - if (!is_null_oid(old_oid) && odb_read_object_info(the_repository->objects, old_oid, &old_bytes) < 0) + if ( + !is_null_oid(old_oid) && + odb_read_object_info(the_repository->objects, old_oid, &old_bytes) < 0 + ) return 1; - if (!is_null_oid(new_oid) && odb_read_object_info(the_repository->objects, new_oid, &new_bytes) < 0) + if ( + !is_null_oid(new_oid) && + odb_read_object_info(the_repository->objects, new_oid, &new_bytes) < 0 + ) return 1; *old_size = old_bytes; @@ -389,7 +425,10 @@ int cgit_diff_files(const struct object_id *old_oid, const struct object_id *new return 1; } - if (buffer_is_binary(old_file.ptr, old_file.size) || buffer_is_binary(new_file.ptr, new_file.size)) { + if ( + buffer_is_binary(old_file.ptr, old_file.size) || + buffer_is_binary(new_file.ptr, new_file.size) + ) { *binary = 1; release_mmfile(&old_file, old_oid); release_mmfile(&new_file, new_oid); @@ -431,6 +470,9 @@ void cgit_diff_tree(const struct object_id *old_oid, const struct object_id *new item = xcalloc(1, sizeof(*item)); item->match = xstrdup(prefix); item->len = strlen(prefix); + // nowildcard_len matching len makes git treat the path as + // literal rather than as a glob. + item->nowildcard_len = item->len; opt.pathspec.nr = 1; opt.pathspec.items = item; } @@ -500,7 +542,7 @@ void cgit_prepare_repo_env(struct cgit_repo *repo) for (i = 0; i < ARRAY_SIZE(vars); i++) if (vars[i].value && setenv(vars[i].name, vars[i].value, 1)) - fprintf(stderr, "[cgit] Error setting env %s=%s: %s (%d)\n", + fprintf(stderr, "[cgit] Unable to set %s=%s: %s (%d)\n", vars[i].name, vars[i].value, strerror(errno), errno); } @@ -508,6 +550,7 @@ int cgit_read_first_line(const char *path, char **buf, size_t *size) { int fd, err; ssize_t got; + size_t want; struct stat st; fd = open(path, O_RDONLY); @@ -522,10 +565,13 @@ int cgit_read_first_line(const char *path, char **buf, size_t *size) close(fd); return EISDIR; } - *buf = xmalloc(st.st_size + 1); - got = read_in_full(fd, *buf, st.st_size); - err = errno; + // Only the first line is wanted, and a description file in a scanned + // repository is whatever a pusher wrote, so the read is bounded. + want = st.st_size < MAX_FIRST_LINE_READ ? st.st_size : MAX_FIRST_LINE_READ; + *buf = xmalloc(want + 1); + got = read_in_full(fd, *buf, want); if (got < 0) { + err = errno; free(*buf); *buf = NULL; *size = 0; @@ -536,12 +582,13 @@ int cgit_read_first_line(const char *path, char **buf, size_t *size) (*buf)[*size] = '\0'; *strchrnul(*buf, '\n') = '\0'; close(fd); - return (*size == (size_t)st.st_size ? 0 : err); + return 0; } char *cgit_strdup_first_line(const char *text) { char *line = xstrdup(text); + *strchrnul(line, '\n') = '\0'; return line; } @@ -561,11 +608,13 @@ char *cgit_expand_macros(const char *text) if (out - start > 0) { *out = '\0'; out = expand_macro(start, limit - start) - 1; + // Step back so the byte that ended the + // token is written again and may open a + // new one. A bare dollar expanded nothing + // and its follower is already in place. + text--; } start = NULL; - // Step back so the byte that ended the token - // is written again and may open a new one. - text--; } out++; text++; @@ -621,6 +670,10 @@ char *cgit_get_mimetype_for_filename(const char *filename) // extension that maps to it. string_list_split_in_place(&fields, line, " \t\r\n", -1); string_list_remove_empty_items(&fields, 0); + if (!fields.nr) { + string_list_clear(&fields, 0); + continue; + } type = fields.items[0].string; for (i = 1; i < fields.nr; i++) { if (!strcasecmp(ext, fields.items[i].string)) { diff --git a/source/shared.h b/source/shared.h index b9f6ecf..9e39389 100644 --- a/source/shared.h +++ b/source/shared.h @@ -32,13 +32,17 @@ extern int cgit_die_unless_non_negative(int result, const char *msg); extern struct cgit_repo *cgit_add_repo(const char *url); extern struct cgit_repo *cgit_get_repoinfo(const char *url); -// Export the CGIT_REPO_* variables that filters and hooks read. +/* + * Export the CGIT_REPO_* variables that filters and hooks read. + */ extern void cgit_prepare_repo_env(struct cgit_repo *repo); extern void cgit_free_reflist_inner(struct reflist *list); -// Matches git's reference iteration callback, and appends each ref it is -// given to the struct reflist passed as cb_data. +/* + * Matches git's reference iteration callback, and appends each ref it is given + * to the struct reflist passed as cb_data. + */ extern int cgit_refs_cb(const struct reference *ref, void *cb_data); extern void cgit_free_commitinfo(struct commitinfo *info); @@ -60,11 +64,15 @@ extern void cgit_diff_tree(const struct object_id *old_oid, const struct object_ filepair_fn fn, const char *prefix, int ignorews); extern void cgit_diff_commit(struct commit *commit, filepair_fn fn, const char *prefix); -// Accept only the date formats cgit documents, leaving mode unchanged for -// anything else. +/* + * Accept only the date formats cgit documents, leaving mode unchanged for + * anything else. + */ extern void cgit_parse_date_format(const char *format, struct date_mode *mode); -// Copy str without its trailing runs of c, returning NULL if nothing is left. +/* + * Copy str without its trailing runs of c, returning NULL if nothing is left. + */ extern char *cgit_trim_end(const char *str, char c); extern char *cgit_ensure_end(const char *str, char c); extern void strbuf_ensure_end(struct strbuf *sb, char c); @@ -84,4 +92,10 @@ extern char *cgit_expand_macros(const char *text); extern char *cgit_get_mimetype_for_filename(const char *filename); +/* + * Whether a revision from the request is a hex object id or a possible ref + * name, the only forms cgit hands to git. + */ +extern int cgit_valid_rev(const char *rev); + #endif // CGIT_SHARED_H diff --git a/tests/t0205-config.sh b/tests/t0205-config.sh index 26231af..4691b8f 100755 --- a/tests/t0205-config.sh +++ b/tests/t0205-config.sh @@ -2,7 +2,7 @@ # Checks what the configuration parser reports on stderr. A key it does not # know, a repo key before any repository and a filter set by a scanned -# repository without trust-scan-filters are each ignored with a warning naming +# repository without trust-scan-config are each ignored with a warning naming # the key, a repo filter in the main cgitrc passes without one, a repository # whose path is missing answers 404 on every page, and a config cgit fully # understands stays silent. @@ -52,12 +52,12 @@ test_expect_success 'a filter in a scanned repository is reported without its fl } >scanrc && CGIT_CONFIG="$PWD/scanrc" QUERY_STRING="url=foo.git/commit/" cgit >tmp 2>err && grep "<div class=.commit-subject.>commit 5" tmp && - grep "Ignoring commit-filter in $PWD/scan/foo.git/: trust-scan-filters is not set" err + grep "Ignoring commit-filter in $PWD/scan/foo.git/: trust-scan-config is not set" err ' test_expect_success 'the flag lets a scanned repository set its filters' ' { - echo "trust-scan-filters=1" && + echo "trust-scan-config=1" && cat scanrc } >scanrc-on && CGIT_CONFIG="$PWD/scanrc-on" QUERY_STRING="url=foo.git/commit/" cgit >tmp 2>err && diff --git a/tests/t0301-security.sh b/tests/t0301-security.sh index 6d6d493..ece60f1 100755 --- a/tests/t0301-security.sh +++ b/tests/t0301-security.sh @@ -242,11 +242,12 @@ test_expect_success 'name-sorted branches are unaffected' ' grep "</html>" tmp ' -# module-link is a template and scan-path lets a repository set its own from -# a cgitrc in the tree, so handing that string to printf let the owner of a -# scanned repository crash the process, or read stack memory into the served -# page, merely by adding conversions past the two that are filled. The tests -# after the fixture also pin down the templates that must keep working. +# module-link is a template and a trusted scan lets a repository set its own +# from a cgitrc in the tree, so handing that string to printf let the owner +# of a scanned repository crash the process, or read stack memory into the +# served page, merely by adding conversions past the two that are filled. +# The tests after the fixture also pin down the templates that must keep +# working. test_expect_success 'set up a submodule fixture with a hostile module-link' ' mkrepo repos/modlink 1 && ( @@ -261,6 +262,7 @@ test_expect_success 'set up a submodule fixture with a hostile module-link' ' { echo "virtual-root=/" && echo "cache-size=0" && + echo "trust-scan-config=1" && echo "scan-path=$PWD/scan" } >modlinkrc ' @@ -313,6 +315,25 @@ test_expect_success 'a doubled percent in a module-link renders as one' ' grep "href=./m/submod/%/$sub." tmp ' +# An unknown page under a repository used to reach the page header with no +# head resolved and crash in the branch switcher. +test_expect_success 'an unknown page under a repository answers 404 with its header' ' + cgit_url "foo/nonsense/" >tmp && + grep "^Status: 404 Not Found" tmp && + grep "Invalid request" tmp && + grep "<select name=.h. " tmp +' + +test_expect_success 'a clone endpoint with http clone off answers the same way' ' + { + echo "enable-http-clone=0" && + cat cgitrc + } >noclonerc && + CGIT_CONFIG="$PWD/noclonerc" QUERY_STRING="url=foo/info/refs" cgit >tmp && + grep "^Status: 404 Not Found" tmp && + grep "<select name=.h. " tmp +' + # The .gitmodules file is commit-controlled content, so a derived link may # carry any scheme and any byte an author can commit. Only http and https # may reach an href, and everything lands attribute-escaped. |