Diff options
| author | Bryce Kwon <bryce@brycekwon.com> | |
|---|---|---|
| committer | Bryce Kwon <bryce@brycekwon.com> | |
| commit | ||
| parent | ||
| tree | ||
| download | ||
Test the advisory classes and name an empty author
Each class behind cgit's published advisories now has a check against
the current code, from a newline in a file name to shell syntax handed
to a filter. The feed also treated an ident with an empty name and an
empty address as present and wrote an empty person, which Atom forbids.
| -rw-r--r-- | source/ui-atom.c | 16 | |
| -rwxr-xr-x | tests/t0301-security.sh | 110 |
2 files changed, 120 insertions, 6 deletions
diff --git a/source/ui-atom.c b/source/ui-atom.c index b536437..39cadb8 100644 --- a/source/ui-atom.c +++ b/source/ui-atom.c @@ -88,9 +88,12 @@ static void print_email(const char *email) if (end) *end = '\0'; - html("<email>"); - xml_txt(start); - html("</email>\n"); + // An empty element is no address at all, which Atom forbids. + if (*start) { + html("<email>"); + xml_txt(start); + html("</email>\n"); + } free(copy); } @@ -111,11 +114,12 @@ static void print_entry(struct commit *commit, const char *host) html("</updated>\n"); html("<author>\n"); // A person construct must hold a name, so a nameless commit falls - // back to the address and then to a placeholder. + // back to the address and then to a placeholder. An ident may carry + // an empty name and an empty address, which count as missing. html("<name>"); - if (info->author) + if (info->author && *info->author) xml_txt(info->author); - else if (info->author_email) + else if (info->author_email && strcmp(info->author_email, "<>")) xml_txt(info->author_email); else html("unknown"); diff --git a/tests/t0301-security.sh b/tests/t0301-security.sh index f1af8e9..1a5ee44 100755 --- a/tests/t0301-security.sh +++ b/tests/t0301-security.sh @@ -373,4 +373,114 @@ test_expect_success 'a quote in a web url cannot break out of the href' ' grep "href=.https://example.com/x'><script>" tmp ' +# The classes behind cgit's published advisories, each pinned against the +# current code: a newline in a file name splitting the headers, a posted +# length overflowing its buffer, a path climbing out on the dumb transport +# and the about page, a commit with an empty author, a percent sign with no +# digits behind it, script in a file name shown by the diff, and shell +# syntax in a file name handed to a filter. +test_expect_success 'set up the advisory fixtures' ' + mkrepo repos/cve 1 && + ( + cd repos/cve && + printf "x\n" >"$(printf "crlf\r\nX-Injected: 1.txt")" && + printf "x\n" >"<script>alert(1)x.txt" && + printf "x\n" >"\$(touch pwned).c" && + git add -A && + git commit -m names && + tree=$(git rev-parse HEAD^{tree}) && + printf "tree %s\nparent %s\n" "$tree" "$(git rev-parse HEAD)" >raw && + printf "author <> 1735689600 +0000\ncommitter <> 1735689600 +0000\n\nempty author\n" >>raw && + cid=$(git hash-object --literally -t commit -w raw) && + git update-ref refs/heads/noname "$cid" + ) && + mkdir -p readme && + printf "readme text\n" >readme/README && + cat >auth.sh <<-\EOF && + #!/bin/sh + case "$1" in + authenticate-post) + cat >/dev/null + printf "Status: 302 Found\nLocation: /\n\n" + ;; + esac + exit 1 + EOF + chmod +x auth.sh && + { + echo "virtual-root=/" && + echo "cache-size=0" && + echo "auth-filter=exec:$PWD/auth.sh" && + echo "repo.url=cve" && + echo "repo.path=$PWD/repos/cve/.git" && + echo "repo.readme=$PWD/readme/README" && + echo "repo.source-filter=exec:$FILTER_DIRECTORY/dump.sh" + } >cverc +' + +cveq() { CGIT_CONFIG="$PWD/cverc" QUERY_STRING="$1" cgit; } + +test_expect_success 'a newline in a file name cannot split the headers' ' + cveq "url=cve/plain/crlf%0d%0aX-Injected:%201.txt" >tmp && + grep "^Status: 200" tmp && + ! grep "^X-Injected" tmp && + grep "^Content-Disposition: inline; filename=.crlf\\\\r\\\\nX-Injected: 1.txt.$" tmp +' + +test_expect_success 'a posted length past any buffer is clamped' ' + echo "username=a&password=b" | + CGIT_CONFIG="$PWD/cverc" REQUEST_METHOD=POST CONTENT_LENGTH=99999999999999 \ + QUERY_STRING="p=login" cgit >tmp && + grep "^Status: 302" tmp +' + +test_expect_success 'the dumb transport refuses a path that climbs out' ' + cveq "url=cve/objects/../config" >tmp && + grep "^Status: 400" tmp && + ! grep "repositoryformatversion" tmp && + cveq "url=cve/objects/info/../../config" >tmp && + grep "^Status: 400" tmp && + ! grep "repositoryformatversion" tmp +' + +test_expect_success 'the about page serves nothing above the readme directory' ' + cveq "url=cve/about/../cverc" >tmp && + grep "^Status: 200" tmp && + ! grep "virtual-root" tmp && + cveq "url=cve/about/" >tmp && + grep "readme text" tmp +' + +test_expect_success 'a commit with an empty author renders everywhere' ' + cveq "url=cve/log/&h=noname" >tmp && + grep ">empty author</a>" tmp && + cveq "url=cve/commit/&h=noname" >tmp && + grep "<div class=.commit-subject.>empty author<" tmp && + cveq "url=cve/atom/&h=noname" >tmp && + grep "<name>unknown</name>" tmp && + ! grep "<email></email>" tmp +' + +test_expect_success 'a percent sign without digits behind it is read as itself' ' + cveq "url=cve/log/&q=%" >tmp && + grep "^Status: 200" tmp && + cveq "url=cve/log/&q=%zz%2" >tmp && + grep "^Status: 200" tmp && + grep "value=.%zz%2." tmp +' + +test_expect_success 'script in a file name is escaped on the diff page' ' + cveq "url=cve/diff/" >tmp && + ! grep "<script>alert" tmp && + grep "<script>alert(1)x.txt" tmp +' + +test_expect_success 'shell syntax in a file name reaches a filter as a plain argument' ' + cveq "url=cve/tree/%24(touch%20pwned).c" >tmp && + grep "^Status: 200" tmp && + grep "pwned).c" tmp && + ! test -e pwned && + ! test -e repos/cve/pwned +' + test_done |