summaryrefslogtreecommitdiff
path: root/t/t9300-fast-import.sh
diff options
context:
space:
mode:
authorJeff King <peff@peff.net>2019-08-29 18:37:26 (GMT)
committerJohannes Schindelin <johannes.schindelin@gmx.de>2019-12-04 12:20:04 (GMT)
commit68061e3470210703cb15594194718d35094afdc0 (patch)
treea4679acf8d204e2d65aa72d28da8667014bdc98f /t/t9300-fast-import.sh
parent019683025f1b14d7cb671312ab01f7330e9b33e7 (diff)
downloadgit-68061e3470210703cb15594194718d35094afdc0.zip
git-68061e3470210703cb15594194718d35094afdc0.tar.gz
git-68061e3470210703cb15594194718d35094afdc0.tar.bz2
fast-import: disallow "feature export-marks" by default
The fast-import stream command "feature export-marks=<path>" lets the stream write marks to an arbitrary path. This may be surprising if you are running fast-import against an untrusted input (which otherwise cannot do anything except update Git objects and refs). Let's disallow the use of this feature by default, and provide a command-line option to re-enable it (you can always just use the command-line --export-marks as well, but the in-stream version provides an easy way for exporters to control the process). This is a backwards-incompatible change, since the default is flipping to the new, safer behavior. However, since the main users of the in-stream versions would be import/export-based remote helpers, and since we trust remote helpers already (which are already running arbitrary code), we'll pass the new option by default when reading a remote helper's stream. This should minimize the impact. Note that the implementation isn't totally simple, as we have to work around the fact that fast-import doesn't parse its command-line options until after it has read any "feature" lines from the stream. This is how it lets command-line options override in-stream. But in our case, it's important to parse the new --allow-unsafe-features first. There are three options for resolving this: 1. Do a separate "early" pass over the options. This is easy for us to do because there are no command-line options that allow the "unstuck" form (so there's no chance of us mistaking an argument for an option), though it does introduce a risk of incorrect parsing later (e.g,. if we convert to parse-options). 2. Move the option parsing phase back to the start of the program, but teach the stream-reading code never to override an existing value. This is tricky, because stream "feature" lines override each other (meaning we'd have to start tracking the source for every option). 3. Accept that we might parse a "feature export-marks" line that is forbidden, as long we don't _act_ on it until after we've parsed the command line options. This would, in fact, work with the current code, but only because the previous patch fixed the export-marks parser to avoid touching the filesystem. So while it works, it does carry risk of somebody getting it wrong in the future in a rather subtle and unsafe way. I've gone with option (1) here as simple, safe, and unlikely to cause regressions. This fixes CVE-2019-1348. Signed-off-by: Jeff King <peff@peff.net>
Diffstat (limited to 't/t9300-fast-import.sh')
-rwxr-xr-xt/t9300-fast-import.sh23
1 files changed, 15 insertions, 8 deletions
diff --git a/t/t9300-fast-import.sh b/t/t9300-fast-import.sh
index 1ba20c1..ba5a35c 100755
--- a/t/t9300-fast-import.sh
+++ b/t/t9300-fast-import.sh
@@ -2117,6 +2117,11 @@ test_expect_success 'R: only one import-marks feature allowed per stream' '
test_must_fail git fast-import <input
'
+test_expect_success 'R: export-marks feature forbidden by default' '
+ echo "feature export-marks=git.marks" >input &&
+ test_must_fail git fast-import <input
+'
+
test_expect_success 'R: export-marks feature results in a marks file being created' '
cat >input <<-EOF &&
feature export-marks=git.marks
@@ -2127,7 +2132,7 @@ test_expect_success 'R: export-marks feature results in a marks file being creat
EOF
- git fast-import <input &&
+ git fast-import --allow-unsafe-features <input &&
grep :1 git.marks
'
@@ -2140,7 +2145,8 @@ test_expect_success 'R: export-marks options can be overridden by commandline op
hi
EOF
- git fast-import --export-marks=cmdline-sub/other.marks <input &&
+ git fast-import --allow-unsafe-features \
+ --export-marks=cmdline-sub/other.marks <input &&
grep :1 cmdline-sub/other.marks &&
test_path_is_missing feature-sub
'
@@ -2148,7 +2154,7 @@ test_expect_success 'R: export-marks options can be overridden by commandline op
test_expect_success 'R: catch typo in marks file name' '
test_must_fail git fast-import --import-marks=nonexistent.marks </dev/null &&
echo "feature import-marks=nonexistent.marks" |
- test_must_fail git fast-import
+ test_must_fail git fast-import --allow-unsafe-features
'
test_expect_success 'R: import and output marks can be the same file' '
@@ -2253,7 +2259,7 @@ test_expect_success 'R: import to output marks works without any content' '
feature export-marks=marks.new
EOF
- git fast-import <input &&
+ git fast-import --allow-unsafe-features <input &&
test_cmp marks.out marks.new
'
@@ -2263,7 +2269,7 @@ test_expect_success 'R: import marks prefers commandline marks file over the str
feature export-marks=marks.new
EOF
- git fast-import --import-marks=marks.out <input &&
+ git fast-import --import-marks=marks.out --allow-unsafe-features <input &&
test_cmp marks.out marks.new
'
@@ -2276,7 +2282,8 @@ test_expect_success 'R: multiple --import-marks= should be honoured' '
head -n2 marks.out > one.marks &&
tail -n +3 marks.out > two.marks &&
- git fast-import --import-marks=one.marks --import-marks=two.marks <input &&
+ git fast-import --import-marks=one.marks --import-marks=two.marks \
+ --allow-unsafe-features <input &&
test_cmp marks.out combined.marks
'
@@ -2289,7 +2296,7 @@ test_expect_success 'R: feature relative-marks should be honoured' '
mkdir -p .git/info/fast-import/ &&
cp marks.new .git/info/fast-import/relative.in &&
- git fast-import <input &&
+ git fast-import --allow-unsafe-features <input &&
test_cmp marks.new .git/info/fast-import/relative.out
'
@@ -2301,7 +2308,7 @@ test_expect_success 'R: feature no-relative-marks should be honoured' '
feature export-marks=non-relative.out
EOF
- git fast-import <input &&
+ git fast-import --allow-unsafe-features <input &&
test_cmp marks.new non-relative.out
'