Diff options
| -rw-r--r-- | source/ui-shared.c | 42 | |
| -rwxr-xr-x | tests/t0200-security.sh | 71 |
2 files changed, 111 insertions, 2 deletions
diff --git a/source/ui-shared.c b/source/ui-shared.c index 1b65a7f..125a235 100644 --- a/source/ui-shared.c +++ b/source/ui-shared.c @@ -616,6 +616,39 @@ static struct string_list_item *lookup_path(struct string_list *list, return NULL; } +/* + * Expand the %s placeholders in a module-link template and emit the result as + * an attribute value. The template is never handed to printf: a repository can + * supply its own through a repo-local cgitrc, and a surplus conversion would + * then read past the argument list. + */ +static void html_module_link(const char *tmpl, const char **args, int nargs) +{ + struct strbuf sb = STRBUF_INIT; + int used = 0; + + while (*tmpl) { + if (*tmpl != '%') { + strbuf_addch(&sb, *tmpl++); + continue; + } + tmpl++; + if (*tmpl == '%') { + strbuf_addch(&sb, '%'); + tmpl++; + } else if (*tmpl == 's' && used < nargs) { + strbuf_addstr(&sb, args[used++]); + tmpl++; + } else { + // Anything else is not a placeholder this template can + // fill, so keep it verbatim rather than dropping it. + strbuf_addch(&sb, '%'); + } + } + html_attr(sb.buf); + strbuf_release(&sb); +} + void cgit_submodule_link(const char *class, char *path, const char *rev) { struct string_list *list; @@ -641,14 +674,19 @@ void cgit_submodule_link(const char *class, char *path, const char *rev) htmlf("class='%s' ", class); html("href='"); if (item) { - html_attrf(item->util, rev); + // A per-path template names only the submodule commit. + const char *args[] = { rev }; + html_module_link(item->util, args, 1); } else { + const char *args[2]; dir = strrchr(path, '/'); if (dir) dir++; else dir = path; - html_attrf(ctx.repo->module_link, dir, rev); + args[0] = dir; + args[1] = rev; + html_module_link(ctx.repo->module_link, args, 2); } html("'>"); html_txt(path); diff --git a/tests/t0200-security.sh b/tests/t0200-security.sh index de248ca..df60174 100755 --- a/tests/t0200-security.sh +++ b/tests/t0200-security.sh @@ -172,4 +172,75 @@ test_expect_success 'a message-less commit renders without crashing' ' grep "no commit message" tmp ' +# --- A repository cannot supply a printf format string ---------------------- +# module-link is a template, and scan-path lets a repository set it through its +# own cgitrc. Handing it to printf let a repo owner crash the process, or read +# stack memory into the served page, with surplus conversions. +test_expect_success 'set up a submodule fixture with a hostile module-link' ' + mkrepo repos/modlink 1 && + ( + cd repos/modlink && + sub=$(git rev-parse HEAD) && + git update-index --add --cacheinfo 160000,$sub,submod && + git commit -m gitlink + ) && + mkdir -p scan && + cp -R repos/modlink scan/modlink && + printf "module-link=/m/%%s/%%s/%%s/%%s/%%s/%%s/%%s/%%s/%%s/%%s\n" \ + >scan/modlink/.git/cgitrc && + { + echo "virtual-root=/" && + echo "cache-size=0" && + echo "scan-path=$PWD/scan" + } >modlinkrc +' + +test_expect_success 'a surplus conversion does not crash the tree view' ' + CGIT_CONFIG="$PWD/modlinkrc" QUERY_STRING="url=modlink/tree/" cgit >tmp && + grep "ls-mod" tmp +' + +test_expect_success 'a surplus conversion is shown literally, not filled' ' + grep "href=./m/submod/[0-9a-f]*/%s/%s/" tmp +' + +test_expect_success 'a well-formed module-link still takes path and sha1' ' + { + echo "virtual-root=/" && + echo "cache-size=0" && + echo "module-link=/mod/%s/commit/?id=%s" && + echo "repo.url=modlink" && + echo "repo.path=$PWD/repos/modlink/.git" + } >modlinkokrc && + sub=$(git -C repos/modlink rev-parse HEAD~1) && + CGIT_CONFIG="$PWD/modlinkokrc" QUERY_STRING="url=modlink/tree/" cgit >tmp && + grep "href=./mod/submod/commit/?id=$sub." tmp +' + +test_expect_success 'a per-path module-link takes only the sha1' ' + { + echo "virtual-root=/" && + echo "cache-size=0" && + echo "repo.url=modlink" && + echo "repo.path=$PWD/repos/modlink/.git" && + echo "repo.module-link.submod=https://example.com/s/?id=%s" + } >modlinkpathrc && + sub=$(git -C repos/modlink rev-parse HEAD~1) && + CGIT_CONFIG="$PWD/modlinkpathrc" QUERY_STRING="url=modlink/tree/" cgit >tmp && + grep "href=.https://example.com/s/?id=$sub." tmp +' + +test_expect_success 'a doubled percent in a module-link renders as one' ' + { + echo "virtual-root=/" && + echo "cache-size=0" && + echo "module-link=/m/%s/%%/%s" && + echo "repo.url=modlink" && + echo "repo.path=$PWD/repos/modlink/.git" + } >modlinkpctrc && + sub=$(git -C repos/modlink rev-parse HEAD~1) && + CGIT_CONFIG="$PWD/modlinkpctrc" QUERY_STRING="url=modlink/tree/" cgit >tmp && + grep "href=./m/submod/%/$sub." tmp +' + test_done |