mirror of
https://github.com/git/git
synced 2024-10-30 04:01:21 +00:00
a376e37b2c
There's a cross-site scripting problem in gitweb, where it will print
URLs generated by its href() helper without further quoting. This allows
an attacker to point a victim to a specially crafted gitweb URL and
inject arbitrary HTML into the resulting page (which the victim sees as
coming from gitweb).
The base of the URL comes from evaluate_uri(), which pulls the value of
$REQUEST_URI via the CGI module. It tries to strip off $PATH_INFO, but
fails to do so in some cases (including ones that contain special
characters, like "+"). Most of the uses of the URL end up being passed
to "$cgi->a(-href = href())", which will get quoted properly by the CGI
module. But in a few places, we output them ourselves as part of
manually-generated HTML, and whatever was in the original URL will
appear unquoted in the output.
Given that all of the nearby variables placed into this manual HTML
_are_ quoted, it seems like the authors assumed that these URLs would
not need quoting. So it's possible that the bug is actually in
evaluate_uri(), which should be doing a more careful job of stripping
$PATH_INFO. There's some discussion in a comment in that function, as
well as the commit message in 81d3fe9f48 (gitweb: fix wrong base URL
when non-root DirectoryIndex, 2009-02-15). But I'm not sure I understand
it.
Regardless, it's a good idea to quote these values at the point of
insertion into the HTML output:
1. Even if there is a bug in evaluate_uri(), this would give us
belt-and-suspenders protection.
2. evaluate_uri() is only handling the base. Some generated URLs will
also mention arbitrary refs or filenames in the repositories, and
these should be quoted anyway.
3. It should never _hurt_ to quote (and that's what all of the
$cgi->a() calls are doing already).
So there may be further work here, but this patch at least prevents the
XSS vulnerability, and shouldn't make anything worse.
The test here covers the calls in print_feed_meta(), but I manually
audited every call to href() to see how its output was used, and quoted
appropriately. Most of them are esc_attr(), as they're used in tag
attributes, but I used esc_html() when the URLs were printed bare. The
distinction is largely academic, as one is implemented as a wrapper for
the other.
Reported-by: NAKAYAMA DAISUKE <nakyamad@icloud.com>
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
207 lines
5.9 KiB
Bash
Executable file
207 lines
5.9 KiB
Bash
Executable file
#!/bin/sh
|
|
#
|
|
# Copyright (c) 2009 Mark Rada
|
|
#
|
|
|
|
test_description='gitweb as standalone script (parsing script output).
|
|
|
|
This test runs gitweb (git web interface) as a CGI script from the
|
|
commandline, and checks that it produces the correct output, either
|
|
in the HTTP header or the actual script output.'
|
|
|
|
|
|
. ./gitweb-lib.sh
|
|
|
|
# ----------------------------------------------------------------------
|
|
# snapshot file name and prefix
|
|
|
|
cat >>gitweb_config.perl <<\EOF
|
|
|
|
$known_snapshot_formats{'tar'} = {
|
|
'display' => 'tar',
|
|
'type' => 'application/x-tar',
|
|
'suffix' => '.tar',
|
|
'format' => 'tar',
|
|
};
|
|
|
|
$feature{'snapshot'}{'default'} = ['tar'];
|
|
EOF
|
|
|
|
# Call check_snapshot with the arguments "<basename> [<prefix>]"
|
|
#
|
|
# This will check that gitweb HTTP header contains proposed filename
|
|
# as <basename> with '.tar' suffix added, and that generated tarfile
|
|
# (gitweb message body) has <prefix> as prefix for al files in tarfile
|
|
#
|
|
# <prefix> default to <basename>
|
|
check_snapshot () {
|
|
basename=$1
|
|
prefix=${2:-"$1"}
|
|
echo "basename=$basename"
|
|
grep "filename=.*$basename.tar" gitweb.headers >/dev/null 2>&1 &&
|
|
"$TAR" tf gitweb.body >file_list &&
|
|
! grep -v -e "^$prefix$" -e "^$prefix/" -e "^pax_global_header$" file_list
|
|
}
|
|
|
|
test_expect_success setup '
|
|
test_commit first foo &&
|
|
git branch xx/test &&
|
|
FULL_ID=$(git rev-parse --verify HEAD) &&
|
|
SHORT_ID=$(git rev-parse --verify --short=7 HEAD)
|
|
'
|
|
test_debug '
|
|
echo "FULL_ID = $FULL_ID"
|
|
echo "SHORT_ID = $SHORT_ID"
|
|
'
|
|
|
|
test_expect_success 'snapshot: full sha1' '
|
|
gitweb_run "p=.git;a=snapshot;h=$FULL_ID;sf=tar" &&
|
|
check_snapshot ".git-$SHORT_ID"
|
|
'
|
|
test_debug 'cat gitweb.headers && cat file_list'
|
|
|
|
test_expect_success 'snapshot: shortened sha1' '
|
|
gitweb_run "p=.git;a=snapshot;h=$SHORT_ID;sf=tar" &&
|
|
check_snapshot ".git-$SHORT_ID"
|
|
'
|
|
test_debug 'cat gitweb.headers && cat file_list'
|
|
|
|
test_expect_success 'snapshot: almost full sha1' '
|
|
ID=$(git rev-parse --short=30 HEAD) &&
|
|
gitweb_run "p=.git;a=snapshot;h=$ID;sf=tar" &&
|
|
check_snapshot ".git-$SHORT_ID"
|
|
'
|
|
test_debug 'cat gitweb.headers && cat file_list'
|
|
|
|
test_expect_success 'snapshot: HEAD' '
|
|
gitweb_run "p=.git;a=snapshot;h=HEAD;sf=tar" &&
|
|
check_snapshot ".git-HEAD-$SHORT_ID"
|
|
'
|
|
test_debug 'cat gitweb.headers && cat file_list'
|
|
|
|
test_expect_success 'snapshot: short branch name (master)' '
|
|
gitweb_run "p=.git;a=snapshot;h=master;sf=tar" &&
|
|
ID=$(git rev-parse --verify --short=7 master) &&
|
|
check_snapshot ".git-master-$ID"
|
|
'
|
|
test_debug 'cat gitweb.headers && cat file_list'
|
|
|
|
test_expect_success 'snapshot: short tag name (first)' '
|
|
gitweb_run "p=.git;a=snapshot;h=first;sf=tar" &&
|
|
ID=$(git rev-parse --verify --short=7 first) &&
|
|
check_snapshot ".git-first-$ID"
|
|
'
|
|
test_debug 'cat gitweb.headers && cat file_list'
|
|
|
|
test_expect_success 'snapshot: full branch name (refs/heads/master)' '
|
|
gitweb_run "p=.git;a=snapshot;h=refs/heads/master;sf=tar" &&
|
|
ID=$(git rev-parse --verify --short=7 master) &&
|
|
check_snapshot ".git-master-$ID"
|
|
'
|
|
test_debug 'cat gitweb.headers && cat file_list'
|
|
|
|
test_expect_success 'snapshot: full tag name (refs/tags/first)' '
|
|
gitweb_run "p=.git;a=snapshot;h=refs/tags/first;sf=tar" &&
|
|
check_snapshot ".git-first"
|
|
'
|
|
test_debug 'cat gitweb.headers && cat file_list'
|
|
|
|
test_expect_success 'snapshot: hierarchical branch name (xx/test)' '
|
|
gitweb_run "p=.git;a=snapshot;h=xx/test;sf=tar" &&
|
|
! grep "filename=.*/" gitweb.headers
|
|
'
|
|
test_debug 'cat gitweb.headers'
|
|
|
|
# ----------------------------------------------------------------------
|
|
# forks of projects
|
|
|
|
test_expect_success 'forks: setup' '
|
|
git init --bare foo.git &&
|
|
echo file > file &&
|
|
git --git-dir=foo.git --work-tree=. add file &&
|
|
git --git-dir=foo.git --work-tree=. commit -m "Initial commit" &&
|
|
echo "foo" > foo.git/description &&
|
|
git clone --bare foo.git foo.bar.git &&
|
|
echo "foo.bar" > foo.bar.git/description &&
|
|
git clone --bare foo.git foo_baz.git &&
|
|
echo "foo_baz" > foo_baz.git/description &&
|
|
rm -fr foo &&
|
|
mkdir -p foo &&
|
|
(
|
|
cd foo &&
|
|
git clone --shared --bare ../foo.git foo-forked.git &&
|
|
echo "fork of foo" > foo-forked.git/description
|
|
)
|
|
'
|
|
|
|
test_expect_success 'forks: not skipped unless "forks" feature enabled' '
|
|
gitweb_run "a=project_list" &&
|
|
grep -q ">\\.git<" gitweb.body &&
|
|
grep -q ">foo\\.git<" gitweb.body &&
|
|
grep -q ">foo_baz\\.git<" gitweb.body &&
|
|
grep -q ">foo\\.bar\\.git<" gitweb.body &&
|
|
grep -q ">foo_baz\\.git<" gitweb.body &&
|
|
grep -q ">foo/foo-forked\\.git<" gitweb.body &&
|
|
grep -q ">fork of .*<" gitweb.body
|
|
'
|
|
|
|
test_expect_success 'enable forks feature' '
|
|
cat >>gitweb_config.perl <<-\EOF
|
|
$feature{"forks"}{"default"} = [1];
|
|
EOF
|
|
'
|
|
|
|
test_expect_success 'forks: forks skipped if "forks" feature enabled' '
|
|
gitweb_run "a=project_list" &&
|
|
grep -q ">\\.git<" gitweb.body &&
|
|
grep -q ">foo\\.git<" gitweb.body &&
|
|
grep -q ">foo_baz\\.git<" gitweb.body &&
|
|
grep -q ">foo\\.bar\\.git<" gitweb.body &&
|
|
grep -q ">foo_baz\\.git<" gitweb.body &&
|
|
grep -v ">foo/foo-forked\\.git<" gitweb.body &&
|
|
grep -v ">fork of .*<" gitweb.body
|
|
'
|
|
|
|
test_expect_success 'forks: "forks" action for forked repository' '
|
|
gitweb_run "p=foo.git;a=forks" &&
|
|
grep -q ">foo/foo-forked\\.git<" gitweb.body &&
|
|
grep -q ">fork of foo<" gitweb.body
|
|
'
|
|
|
|
test_expect_success 'forks: can access forked repository' '
|
|
gitweb_run "p=foo/foo-forked.git;a=summary" &&
|
|
grep -q "200 OK" gitweb.headers &&
|
|
grep -q ">fork of foo<" gitweb.body
|
|
'
|
|
|
|
test_expect_success 'forks: project_index lists all projects (incl. forks)' '
|
|
cat >expected <<-\EOF &&
|
|
.git
|
|
foo.bar.git
|
|
foo.git
|
|
foo/foo-forked.git
|
|
foo_baz.git
|
|
EOF
|
|
gitweb_run "a=project_index" &&
|
|
sed -e "s/ .*//" <gitweb.body | sort >actual &&
|
|
test_cmp expected actual
|
|
'
|
|
|
|
xss() {
|
|
echo >&2 "Checking $*..." &&
|
|
gitweb_run "$@" &&
|
|
if grep "$TAG" gitweb.body; then
|
|
echo >&2 "xss: $TAG should have been quoted in output"
|
|
return 1
|
|
fi
|
|
return 0
|
|
}
|
|
|
|
test_expect_success 'xss checks' '
|
|
TAG="<magic-xss-tag>" &&
|
|
xss "a=rss&p=$TAG" &&
|
|
xss "a=rss&p=foo.git&f=$TAG" &&
|
|
xss "" "$TAG+"
|
|
'
|
|
|
|
test_done
|