Diff options
| -rw-r--r-- | source/ui-refs.c | 22 | |
| -rwxr-xr-x | tests/t0200-security.sh | 57 |
2 files changed, 66 insertions, 13 deletions
diff --git a/source/ui-refs.c b/source/ui-refs.c index 932ba6d..84b929c 100644 --- a/source/ui-refs.c +++ b/source/ui-refs.c @@ -27,14 +27,6 @@ static int cmp_ref_name(const void *a, const void *b) return strcmp(r1->refname, r2->refname); } -static int cmp_branch_age(const void *a, const void *b) -{ - struct refinfo *r1 = *(struct refinfo **)a; - struct refinfo *r2 = *(struct refinfo **)b; - - return cmp_age(r1->commit->committer_date, r2->commit->committer_date); -} - static int get_ref_age(struct refinfo *ref) { if (!ref->object) @@ -48,7 +40,11 @@ static int get_ref_age(struct refinfo *ref) return 0; } -static int cmp_tag_age(const void *a, const void *b) +// tag and commit share a union in struct refinfo and only the member matching +// the object type is ever filled, so the date has to be reached through +// get_ref_age rather than by assuming a branch points at a commit. Reading the +// wrong member ran off the end of the smaller struct. +static int cmp_ref_age(const void *a, const void *b) { struct refinfo *r1 = *(struct refinfo **)a; struct refinfo *r2 = *(struct refinfo **)b; @@ -203,7 +199,7 @@ void cgit_print_branches(int maxcount) if (maxcount == 0 || maxcount > list.count) maxcount = list.count; - qsort(list.refs, list.count, sizeof(*list.refs), cmp_branch_age); + qsort(list.refs, list.count, sizeof(*list.refs), cmp_ref_age); if (ctx.repo->branch_sort == 0) qsort(list.refs, maxcount, sizeof(*list.refs), cmp_ref_name); @@ -227,7 +223,7 @@ void cgit_print_tags(int maxcount) cgit_refs_cb, &list); if (list.count == 0) return; - qsort(list.refs, list.count, sizeof(*list.refs), cmp_tag_age); + qsort(list.refs, list.count, sizeof(*list.refs), cmp_ref_age); if (!maxcount) maxcount = list.count; else if (maxcount > list.count) @@ -253,7 +249,7 @@ static void print_branches_page(int pagesize) print_branch_header(); collect_branches(&list); - qsort(list.refs, list.count, sizeof(*list.refs), cmp_branch_age); + qsort(list.refs, list.count, sizeof(*list.refs), cmp_ref_age); if (ctx.repo->branch_sort == 0) qsort(list.refs, list.count, sizeof(*list.refs), cmp_ref_name); @@ -284,7 +280,7 @@ static void print_tags_page(int pagesize) cgit_refs_cb, &list); if (list.count == 0) return; - qsort(list.refs, list.count, sizeof(*list.refs), cmp_tag_age); + qsort(list.refs, list.count, sizeof(*list.refs), cmp_ref_age); if (pagesize <= 0 || pagesize > list.count) pagesize = list.count; diff --git a/tests/t0200-security.sh b/tests/t0200-security.sh index df60174..9238262 100755 --- a/tests/t0200-security.sh +++ b/tests/t0200-security.sh @@ -172,6 +172,63 @@ test_expect_success 'a message-less commit renders without crashing' ' grep "no commit message" tmp ' +# --- A branch need not point at a commit ------------------------------------ +# struct refinfo keeps taginfo and commitinfo in a union and fills only the one +# matching the object type. Sorting branches by reading the commit member read +# past the end of the smaller taginfo, and read NULL for a tree, which crashed. +# git update-ref refuses to write these, so the refs go in as loose files: a +# repository is just files on disk and cgit reads whatever is there. +test_expect_success 'set up a repo whose branches point at odd objects' ' + mkrepo repos/oddref 2 && + ( + cd repos/oddref && + git tag -a annotated -m note && + git rev-parse annotated >.git/refs/heads/points-at-tag && + git rev-parse HEAD^{tree} >.git/refs/heads/points-at-tree && + git for-each-ref refs/heads/ >refs.out && + grep -q "tree.refs/heads/points-at-tree" refs.out && + grep -q "tag.refs/heads/points-at-tag" refs.out + ) && + { + echo "virtual-root=/" && + echo "cache-size=0" && + echo "branch-sort=age" && + echo "repo.url=oddref" && + echo "repo.path=$PWD/repos/oddref/.git" + } >oddrefrc +' + +test_expect_success 'refs page sorts such branches without crashing' ' + CGIT_CONFIG="$PWD/oddrefrc" QUERY_STRING="url=oddref/refs/" cgit >tmp && + grep "points-at-tree" tmp && + grep "</html>" tmp +' + +test_expect_success 'the branch page sorts them without crashing' ' + CGIT_CONFIG="$PWD/oddrefrc" QUERY_STRING="url=oddref/refs/heads/" cgit >tmp && + grep "points-at-tree" tmp && + grep "</html>" tmp +' + +test_expect_success 'the summary page sorts them without crashing' ' + CGIT_CONFIG="$PWD/oddrefrc" QUERY_STRING="url=oddref/" cgit >tmp && + grep "points-at-tree" tmp && + grep "</html>" tmp +' + +test_expect_success 'name-sorted branches are unaffected' ' + { + echo "virtual-root=/" && + echo "cache-size=0" && + echo "branch-sort=name" && + echo "repo.url=oddref" && + echo "repo.path=$PWD/repos/oddref/.git" + } >oddrefnamerc && + CGIT_CONFIG="$PWD/oddrefnamerc" QUERY_STRING="url=oddref/refs/heads/" cgit >tmp && + grep "points-at-tree" tmp && + grep "</html>" 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 |