From: Omar Polo Subject: gotwebd: percent-encode querystrings To: gameoftrees@openbsd.org Date: Sat, 03 Sep 2022 11:27:58 +0200 TL;DR: wanna break gotwebd? $ touch "' onclick='alert(\"p0wn3d\");' foo='" $ got add -R . $ got commit -m ... $ got send enjoy! (this doesn't work on modern browsers due to the CSP, but gives the idea.) Longer version: gotwebd doesn't encode the arguments in the generated URLs. This is a companion diff to the other diff i've sent ("gotwebd: urldecode querystring") and deals with how URLs are generated. Some URL parameters needs to be encoded as they contain data that may otherwise break the HTML. Among these `folder', `file' and `path'. Instead of making the current URL construction even more complex, I decided to take it as a chance to refactor a bit how we generate URLs. Diff below introduces a new helper: `gotweb_link'. It takes the request, a struct gotweb_url that represent the request, followed by a printf-format string and arguments. It makes gotwebd gain a few lines of codes because now the various fcgi_printf calls needs to be splitted, but otherwise I think it's an improvement. What do y'all think? Is it OK to use the syntax r = gotweb_link(c, &(struct gotweb_url){ .index_page = -1, .action = DIFF, .page = -1, .path = repo_dir->name, .commit = bline->id_str, }, "%.8s", bline->id_str); to pass an anounymous struct pointer? diff /home/op/w/got commit - e5e662e42c45f0d30f5f97fb0e2ad5f3c4f8b488 path + /home/op/w/got blob - e77b8424231bf88c153a32dd90a19292c41dc39d file + gotwebd/fcgi.c --- gotwebd/fcgi.c +++ gotwebd/fcgi.c @@ -288,16 +288,12 @@ fcgi_timeout(int fd, short events, void *arg) } int -fcgi_printf(struct request *c, const char *fmt, ...) +fcgi_vprintf(struct request *c, const char *fmt, va_list ap) { - va_list ap; char *str; int r; - va_start(ap, fmt); r = vasprintf(&str, fmt, ap); - va_end(ap); - if (r == -1) { log_warn("%s: asprintf", __func__); return -1; @@ -309,6 +305,19 @@ fcgi_printf(struct request *c, const char *fmt, ...) } int +fcgi_printf(struct request *c, const char *fmt, ...) +{ + va_list ap; + int r; + + va_start(ap, fmt); + r = fcgi_vprintf(c, fmt, ap); + va_end(ap); + + return r; +} + +int fcgi_gen_binary_response(struct request *c, const uint8_t *data, int len) { int r; blob - 14d7cbf7261ca9171a51aa4286a2d0870ffcf937 file + gotwebd/got_operations.c --- gotwebd/got_operations.c +++ gotwebd/got_operations.c @@ -882,42 +882,76 @@ got_output_repo_tree(struct request *c) goto done; if (S_ISDIR(mode)) { - r = fcgi_printf(c, - "
\n" - "
" - "%s%s" - "
\n" /* .tree_line */ + struct gotweb_url url = { + .index_page = -1, + .page = -1, + .action = TREE, + .commit = rc->commit_id, + .path = qs->path, + /* folder filled later */ + }; + char *path = NULL; + + if (fcgi_printf(c,"
\n" + "
") == -1) + goto done; + + if (asprintf(&path, "%s/%s", folder, name) == -1) { + error = got_error_from_errno("asprintf"); + goto done; + } + url.folder = path; + r = gotweb_link(c, &url, "%s%s", escaped_name, + modestr); + free(path); + if (r == -1) + goto done; + + if (fcgi_printf(c, "
\n" /* .tree_line */ "
 
\n" - "
\n", /* .tree_wrapper */ - index_page_str, qs->path, rc->commit_id, - folder, name, escaped_name, modestr); - if (r == -1) + "
\n") == -1) goto done; } else { - r = fcgi_printf(c, - "
\n" - "
" - "%s%s" - "
\n" /* .tree_line */ - "
" - "commits\n" - " | \n" - "blame\n" - "
\n" /* .tree_line_blank */ - "
\n", /* .tree_wrapper */ - index_page_str, qs->path, rc->commit_id, - folder, name, escaped_name, modestr, - index_page_str, qs->path, rc->commit_id, - folder, name, - index_page_str, qs->path, rc->commit_id, - folder, name); + struct gotweb_url url = { + .index_page = -1, + .page = -1, + .path = qs->path, + .commit = rc->commit_id, + .folder = folder, + .file = name, + }; + + if (fcgi_printf(c, "
\n" + "
") == -1) + goto done; + + url.action = BLOB; + r = gotweb_link(c, &url, "%s%s", escaped_name, + modestr); if (r == -1) goto done; + + if (fcgi_printf(c, "
\n" /* .tree_line */ + "
") == -1) + goto done; + + url.action = COMMITS; + r = gotweb_link(c, &url, "commits"); + if (r == -1) + goto done; + + if (fcgi_printf(c, " | ") == -1) + goto done; + + url.action = BLAME; + r = gotweb_link(c, &url, "blame"); + if (r == -1) + goto done; + + if (fcgi_printf(c, + "
\n" /* .tree_line_blank */ + "
\n") == -1) /* .tree_wrapper */ + goto done; } free(escaped_name); escaped_name = NULL; @@ -1155,19 +1189,28 @@ got_gotweb_blame_cb(void *arg, int nlines, int lineno, if (err) goto done; - r = fcgi_printf(c, "
" + if (fcgi_printf(c, "
" "
%.*d
" - "
" - "" - "%.8s" + "
", + a->nlines_prec, a->lineno_cur) == -1) + goto done; + + r = gotweb_link(c, &(struct gotweb_url){ + .index_page = -1, + .action = DIFF, + .page = -1, + .path = repo_dir->name, + .commit = bline->id_str, + }, "%.8s", bline->id_str); + if (r == -1) + goto done; + + r = fcgi_printf(c, "
" "
%s
" "
%s
" "
%s
" "
", /* .blame_wrapper */ - a->nlines_prec, a->lineno_cur, - index_page_str, repo_dir->name, bline->id_str, - bline->id_str, bline->datebuf, committer, eline); blob - 00c0cad7a1dfa41e7edf76bc95925af1573138c8 (staged) file + gotwebd/gotweb.c --- gotwebd/gotweb.c +++ gotwebd/gotweb.c @@ -669,6 +669,7 @@ gotweb_render_content_type_file(struct request *c, con static const struct got_error * gotweb_render_header(struct request *c) { + const struct got_error *err = NULL; struct server *srv = c->srv; struct querystring *qs = c->t->qs; int r; @@ -718,10 +719,22 @@ gotweb_render_header(struct request *c) if (qs != NULL) { if (qs->path != NULL) { - r = fcgi_printf(c, " / " - "" - "%s", - qs->index_page, qs->path, qs->path); + char *epath; + + if (fcgi_printf(c, " / ") == -1) + goto done; + + err = gotweb_escape_html(&epath, qs->path); + if (err) + return err; + r = gotweb_link(c, &(struct gotweb_url){ + .index_page = -1, + .page = -1, + .action = SUMMARY, + .path = qs->path, + }, "%s", epath); + free(epath); + if (r == -1) goto done; } @@ -801,8 +814,7 @@ gotweb_render_navs(struct request *c) struct transport *t = c->t; struct querystring *qs = t->qs; struct server *srv = c->srv; - char *nhref = NULL, *phref = NULL; - int r, disp = 0; + int r; r = fcgi_printf(c, "
\n\n" /* #nav_prev */ "\n"); /* #nav_next */ fcgi_printf(c, "
\n"); /* #np_wrapper */ done: @@ -952,8 +958,6 @@ done: t->next_id = NULL; free(t->prev_id); t->prev_id = NULL; - free(phref); - free(nhref); return error; } @@ -1066,17 +1070,22 @@ render: d_disp++; t->prev_disp++; - r = fcgi_printf(c, "
\n" - "
" - "" - " %s " - "" - "
", /* .index_project */ - index_page_str, repo_dir->name, - repo_dir->name); + if (fcgi_printf(c, "
\n" + "
") == -1) + goto done; + + r = gotweb_link(c, &(struct gotweb_url){ + .index_page = -1, + .page = -1, + .action = SUMMARY, + .path = repo_dir->name, + }, "%s", repo_dir->name); if (r == -1) goto done; + if (fcgi_printf(c, "
") == -1) /* .index_project */ + goto done; + if (srv->show_repo_description) { r = fcgi_printf(c, "
\n" @@ -1099,32 +1108,71 @@ render: goto done; } - r = fcgi_printf(c, "
" - "
" - "" - "summary" - " | " - "" - "commit briefs" - " | " - "" - "commits" - " | " - "" - "tags" - " | " - "" - "tree" - "" - "
" /* .navs */ + if (fcgi_printf(c, "
" + "
") == -1) + goto done; + + r = gotweb_link(c, &(struct gotweb_url){ + .index_page = -1, + .page = -1, + .action = SUMMARY, + .path = repo_dir->name + }, "summary"); + if (r == -1) + goto done; + + if (fcgi_printf(c, " | ") == -1) + goto done; + + r = gotweb_link(c, &(struct gotweb_url){ + .index_page = -1, + .page = -1, + .action = BRIEFS, + .path = repo_dir->name + }, "commit briefs"); + if (r == -1) + goto done; + + if (fcgi_printf(c, " | ") == -1) + goto done; + + r = gotweb_link(c, &(struct gotweb_url){ + .index_page = -1, + .page = -1, + .action = COMMITS, + .path = repo_dir->name + }, "commits"); + if (r == -1) + goto done; + + if (fcgi_printf(c, " | ") == -1) + goto done; + + r = gotweb_link(c, &(struct gotweb_url){ + .index_page = -1, + .page = -1, + .action = TAGS, + .path = repo_dir->name + }, "tags"); + if (r == -1) + goto done; + + if (fcgi_printf(c, " | ") == -1) + goto done; + + r = gotweb_link(c, &(struct gotweb_url){ + .index_page = -1, + .page = -1, + .action = TREE, + .path = repo_dir->name + }, "tree"); + if (r == -1) + goto done; + + r = fcgi_printf(c, "
" /* .navs */ "
\n" - "
\n" /* .navs_wrapper */ - "
\n", /* .index_wrapper */ - index_page_str, repo_dir->name, - index_page_str, repo_dir->name, - index_page_str, repo_dir->name, - index_page_str, repo_dir->name, - index_page_str, repo_dir->name); + "
\n" /* .navs_wrapper */ + "
\n"); /* .index_wrapper */ if (r == -1) goto done; @@ -1262,16 +1310,22 @@ gotweb_render_briefs(struct request *c) r = fcgi_printf(c, "
%s
\n" "
%s
\n" - "
" - "%s", - age, - author, - index_page_str, repo_dir->name, rc->commit_id, qs->headref, - msg); + "
", + age, author); if (r == -1) goto done; + r = gotweb_link(c, &(struct gotweb_url){ + .index_page = -1, + .action = DIFF, + .page = -1, + .path = repo_dir->name, + .commit = rc->commit_id, + .headref = qs->headref, + }, "%s", msg); + if (r == -1) + goto done; + if (rc->refs_str) { char *refs; @@ -1288,18 +1342,38 @@ gotweb_render_briefs(struct request *c) goto done; r = fcgi_printf(c, "
\n" - "
" - "diff" - " | " - "tree" - "
\n" /* .navs */ + "
"); + if (r == -1) + goto done; + + r = gotweb_link(c, &(struct gotweb_url){ + .index_page = -1, + .page = -1, + .action = DIFF, + .path = repo_dir->name, + .commit = rc->commit_id, + .headref = qs->headref, + }, "diff"); + if (r == -1) + goto done; + + if (fcgi_printf(c, " | ") == -1) + goto done; + + r = gotweb_link(c, &(struct gotweb_url){ + .index_page = -1, + .page = -1, + .action = TREE, + .path = repo_dir->name, + .commit = rc->commit_id, + .headref = qs->headref, + }, "tree"); + if (r == -1) + goto done; + + if (fcgi_printf(c, "
\n" /* .navs */ "
\n" /* .navs_wrapper */ - "
\n", - index_page_str, repo_dir->name, rc->commit_id, qs->headref, - index_page_str, repo_dir->name, rc->commit_id, qs->headref); - if (r == -1) + "
\n") == -1) goto done; free(age); @@ -1379,19 +1453,36 @@ gotweb_render_commits(struct request *c) if (r == -1) goto done; - r = fcgi_printf(c, "
\n" - "
" - "" - "diff" - " | " - "" - "tree" - "
\n" /* .navs */ + if (fcgi_printf(c, "
\n" + "
") == -1) + goto done; + + r = gotweb_link(c, &(struct gotweb_url){ + .index_page = -1, + .page = -1, + .action = DIFF, + .path = repo_dir->name, + .commit = rc->commit_id, + }, "diff"); + if (r == -1) + goto done; + + if (fcgi_printf(c, " | ") == -1) + goto done; + + r = gotweb_link(c, &(struct gotweb_url){ + .index_page = -1, + .page = -1, + .action = TREE, + .path = repo_dir->name, + .commit = rc->commit_id, + }, "tree"); + if (r == -1) + goto done; + + if (fcgi_printf(c, "
\n" /* .navs */ "
\n" /* .navs_wrapper */ - "
\n", - index_page_str, repo_dir->name, rc->commit_id, - index_page_str, repo_dir->name, rc->commit_id); - if (r == -1) + "
\n") == -1) goto done; free(age); @@ -1424,12 +1515,10 @@ gotweb_render_branches(struct request *c) struct transport *t = c->t; struct querystring *qs = t->qs; struct got_repository *repo = t->repo; - const char *index_page_str; + char *escaped_refname = NULL; char *age = NULL; int r; - index_page_str = qs->index_page_str ? qs->index_page_str : ""; - TAILQ_INIT(&refs); error = got_ref_list(&refs, repo, "refs/heads", @@ -1446,7 +1535,6 @@ gotweb_render_branches(struct request *c) TAILQ_FOREACH(re, &refs, entry) { const char *refname = NULL; - char *escaped_refname = NULL; if (got_ref_is_symbolic(re->ref)) continue; @@ -1473,41 +1561,77 @@ gotweb_render_branches(struct request *c) r = fcgi_printf(c, "
\n" "
%s
\n" "
 
\n" - "
" - "" - "%s" - "
\n" /* .branch */ + "
", age); + if (r == -1) + goto done; + + r = gotweb_link(c, &(struct gotweb_url){ + .index_page = -1, + .page = -1, + .action = SUMMARY, + .path = qs->path, + .headref = refname, + }, "%s", escaped_refname); + if (r == -1) + goto done; + + if (fcgi_printf(c, "
\n" /* .branch */ "
\n" - "
" - "" - "summary" - " | " - "" - "commit briefs" - " | " - "" - "commits" - "
\n" /* .navs */ - "
\n" /* .navs_wrapper */ + "
") == -1) + goto done; + + r = gotweb_link(c, &(struct gotweb_url){ + .index_page = -1, + .page = -1, + .action = SUMMARY, + .path = qs->path, + .headref = refname, + }, "summary"); + if (r == -1) + goto done; + + if (fcgi_printf(c, " | ") == -1) + goto done; + + r = gotweb_link(c, &(struct gotweb_url){ + .index_page = -1, + .page = -1, + .action = BRIEFS, + .path = qs->path, + .headref = refname, + }, "commit briefs"); + if (r == -1) + goto done; + + if (fcgi_printf(c, " | ") == -1) + goto done; + + r = gotweb_link(c, &(struct gotweb_url){ + .index_page = -1, + .page = -1, + .action = COMMITS, + .path = qs->path, + .headref = refname, + }, "commits"); + if (r == -1) + goto done; + + r = fcgi_printf(c, "
\n" /* .navs */ + "
\n" /* .navs_wrapper */ "
\n" - "
\n", /* .branches_wrapper */ - age ? age : "", - index_page_str, qs->path, refname, - escaped_refname, - index_page_str, qs->path, refname, - index_page_str, qs->path, refname, - index_page_str, qs->path, refname); + "
\n"); /* .branches_wrapper */ + if (r == -1) + goto done; + + free(age); + age = NULL; free(escaped_refname); - if (r == -1) - goto done; - - free(age); - age = NULL; - + escaped_refname = NULL; } fcgi_printf(c, "
\n"); /* #branches_content */ done: free(age); + free(escaped_refname); got_ref_list_free(&refs); return error; } @@ -1787,12 +1911,9 @@ gotweb_render_tags(struct request *c) struct transport *t = c->t; struct querystring *qs = t->qs; struct repo_dir *repo_dir = t->repo_dir; - const char *index_page_str; char *age = NULL, *tagname = NULL, *msg = NULL, *newline; int r, commit_found = 0; - index_page_str = qs->index_page_str ? qs->index_page_str : ""; - if (qs->action == BRIEFS) { qs->action = TAGS; error = got_get_repo_tags(c, D_MAXSLCOMMDISP); @@ -1842,32 +1963,66 @@ gotweb_render_tags(struct request *c) goto done; } - r = fcgi_printf(c, "
%s
\n" + if (fcgi_printf(c, "
%s
\n" "
%s
\n" - "
" - "" - "%s" - "
\n" /* .tag_log */ + "
", age, tagname) == -1) + goto done; + + r = gotweb_link(c, &(struct gotweb_url){ + .index_page = -1, + .page = -1, + .action = TAG, + .path = repo_dir->name, + .commit = rt->commit_id, + }, "%s", msg ? msg : ""); + if (r == -1) + goto done; + + if (fcgi_printf(c, "
\n" /* .tag_log */ "
\n" - "
" - "" - "tag" - " | " - "" - "commit briefs" - " | " - "" - "commits" + "
") == -1) + goto done; + + r = gotweb_link(c, &(struct gotweb_url){ + .index_page = -1, + .page = -1, + .action = TAG, + .path = repo_dir->name, + .commit = rt->commit_id, + }, "tag"); + if (r == -1) + goto done; + + if (fcgi_printf(c, " | ") == -1) + goto done; + + r = gotweb_link(c, &(struct gotweb_url){ + .index_page = -1, + .page = -1, + .action = BRIEFS, + .path = repo_dir->name, + .commit = rt->commit_id, + }, "commit briefs"); + if (r == -1) + goto done; + + if (fcgi_printf(c, " | ") == -1) + goto done; + + r = gotweb_link(c, &(struct gotweb_url){ + .index_page = -1, + .page = -1, + .action = COMMITS, + .path = repo_dir->name, + .commit = rt->commit_id, + }, "commits"); + if (r == -1) + goto done; + + r = fcgi_printf(c, "
\n" /* .navs */ "
\n" /* .navs_wrapper */ - "
\n", - age, - tagname, - index_page_str, repo_dir->name, rt->commit_id, - msg ? msg : "", - index_page_str, repo_dir->name, rt->commit_id, - index_page_str, repo_dir->name, rt->commit_id, - index_page_str, repo_dir->name, rt->commit_id); + "
\n"); if (r == -1) goto done; @@ -1955,6 +2110,219 @@ done: return error; } +static inline int +should_urlencode(int c) +{ + if (c <= ' ' || c >= 127) + return 1; + + switch (c) { + /* gen-delim */ + case ':': + case '/': + case '?': + case '#': + case '[': + case ']': + case '@': + /* sub-delims */ + case '!': + case '$': + case '&': + case '\'': + case '(': + case ')': + case '*': + case '+': + case ',': + case ';': + case '=': + return 1; + default: + return 0; + } +} + +static char * +gotweb_urlencode(const char *str) +{ + const char *s; + char *escaped; + size_t i, len; + int a, b; + + len = 0; + for (s = str; *s; ++s) { + len++; + if (should_urlencode(*s)) + len += 2; + } + + escaped = calloc(1, len + 1); + if (escaped == NULL) + return NULL; + + i = 0; + for (s = str; *s; ++s) { + if (should_urlencode(*s)) { + a = (*s & 0xF0) >> 4; + b = (*s & 0x0F); + + escaped[i++] = '%'; + escaped[i++] = a <= 9 ? ('0' + a) : ('7' + a); + escaped[i++] = b <= 9 ? ('0' + b) : ('7' + b); + } else + escaped[i++] = *s; + } + + return escaped; +} + +static inline const char * +action_name(int action) +{ + switch (action) { + case BLAME: + return "blame"; + case BLOB: + return "blob"; + case BRIEFS: + return "briefs"; + case COMMITS: + return "commits"; + case DIFF: + return "diff"; + case ERR: + return "err"; + case INDEX: + return "index"; + case SUMMARY: + return "summary"; + case TAG: + return "tag"; + case TAGS: + return "tags"; + case TREE: + return "tree"; + default: + return NULL; + } +} + +static int +gotweb_print_url(struct request *c, struct gotweb_url *url) +{ + const char *sep = "?", *action; + char *tmp; + char *eip = NULL, *ep = NULL, *ef = NULL, *efile = NULL; + int r; + + action = action_name(url->action); + if (action != NULL) { + if (fcgi_printf(c, "?action=%s", action) == -1) + return -1; + sep = "&"; + } + + if (url->commit) { + if (fcgi_printf(c, "%scommit=%s", sep, url->commit) == -1) + return -1; + sep = "&"; + } + + if (url->previd) { + if (fcgi_printf(c, "%sprevid=%s", sep, url->previd) == -1) + return -1; + sep = "&"; + } + + if (url->prevset) { + if (fcgi_printf(c, "%sprevset=%s", sep, url->prevset) == -1) + return -1; + sep = "&"; + } + + if (url->file) { + tmp = gotweb_urlencode(url->file); + if (tmp == NULL) + return -1; + r = fcgi_printf(c, "%sfile=%s", sep, tmp); + free(tmp); + if (r == -1) + return -1; + sep = "&"; + } + + if (url->folder) { + tmp = gotweb_urlencode(url->folder); + if (tmp == NULL) + return -1; + r = fcgi_printf(c, "%sfolder=%s", sep, tmp); + free(tmp); + if (r == -1) + return -1; + sep = "&"; + } + + if (url->headref) { + if (fcgi_printf(c, "%sheadref=%s", sep, url->headref) == -1) + return -1; + sep = "&"; + } + + if (url->index_page != -1) { + if (fcgi_printf(c, "%sindex_page=%d", sep, + url->index_page) == -1) + return -1; + sep = "&"; + } + + if (url->path) { + tmp = gotweb_urlencode(url->path); + if (tmp == NULL) + return -1; + r = fcgi_printf(c, "%spath=%s", sep, tmp); + free(tmp); + if (r == -1) + return -1; + sep = "&"; + } + + if (url->page != -1) { + if (fcgi_printf(c, "%spage=%d", sep, url->page) == -1) + return -1; + sep = "&"; + } + + return 0; +} + +int +gotweb_link(struct request *c, struct gotweb_url *url, const char *fmt, ...) +{ + va_list ap; + int r; + + if (fcgi_printf(c, "") == -1) + return -1; + + va_start(ap, fmt); + r = fcgi_vprintf(c, fmt, ap); + va_end(ap); + if (r == -1) + return -1; + + if (fcgi_printf(c, "")) + return -1; + return 0; +} + static struct got_repository * find_cached_repo(struct server *srv, const char *path) { blob - ac7f962b1abdcfe1c535e8bc210c9f619e7a2cbc file + gotwebd/gotwebd.h --- gotwebd/gotwebd.h +++ gotwebd/gotwebd.h @@ -341,6 +341,19 @@ struct gotwebd { char unix_socket_name[PATH_MAX]; }; +struct gotweb_url { + int action; + const char *commit; + const char *previd; + const char *prevset; + const char *file; + const char *folder; + const char *headref; + int index_page; + const char *path; + int page; +}; + struct querystring { uint8_t action; char *commit; @@ -411,6 +424,9 @@ const struct got_error const struct got_error *gotweb_get_time_str(char **, time_t, int); const struct got_error *gotweb_init_transport(struct transport **); const struct got_error *gotweb_escape_html(char **, const char *); +int gotweb_link(struct request *, struct gotweb_url *, const char *, ...) + __attribute__((__format__(printf, 3, 4))) + __attribute__((__nonnull__(3))); void gotweb_free_repo_commit(struct repo_commit *); void gotweb_free_repo_tag(struct repo_tag *); void gotweb_process_request(struct request *); @@ -426,6 +442,7 @@ void fcgi_timeout(int, short, void *); void fcgi_cleanup_request(struct request *); void fcgi_create_end_record(struct request *); void dump_fcgi_record(const char *, struct fcgi_record_header *); +int fcgi_vprintf(struct request *, const char *, va_list); int fcgi_printf(struct request *, const char *, ...) __attribute__((__format__(printf, 2, 3))) __attribute__((__nonnull__(2)));