From a52ed76142f6e8d993bb4c50938a408966eb2b7c Mon Sep 17 00:00:00 2001 From: Jeff King Date: Thu, 29 Aug 2019 15:08:42 -0400 Subject: [PATCH] fast-import: disallow "feature import-marks" by default As with export-marks in the previous commit, import-marks can access the filesystem. This is significantly less dangerous than export-marks because it only involves reading from arbitrary paths, rather than writing them. However, it could still be surprising and have security implications (e.g., exfiltrating data from a service that accepts fast-import streams). Let's lump it (and its "if-exists" counterpart) in with export-marks, and enable the in-stream version only if --allow-unsafe-features is set. Signed-off-by: Jeff King --- Documentation/git-fast-import.txt | 3 ++- fast-import.c | 2 ++ t/t9300-fast-import.sh | 22 +++++++++++++++++----- 3 files changed, 21 insertions(+), 6 deletions(-) diff --git a/Documentation/git-fast-import.txt b/Documentation/git-fast-import.txt index fbb3f914f2..ff71fc2962 100644 --- a/Documentation/git-fast-import.txt +++ b/Documentation/git-fast-import.txt @@ -57,7 +57,8 @@ OPTIONS allowing fast-import to access the filesystem outside of the repository). These options are disabled by default, but can be allowed by providing this option on the command line. This - currently impacts only the `feature export-marks` command. + currently impacts only the `export-marks`, `import-marks`, and + `import-marks-if-exists` feature commands. + Only enable this option if you trust the program generating the fast-import stream! This option is enabled automatically for diff --git a/fast-import.c b/fast-import.c index 967077ad0b..93c3838254 100644 --- a/fast-import.c +++ b/fast-import.c @@ -3344,8 +3344,10 @@ static int parse_one_feature(const char *feature, int from_stream) if (skip_prefix(feature, "date-format=", &arg)) { option_date_format(arg); } else if (skip_prefix(feature, "import-marks=", &arg)) { + check_unsafe_feature("import-marks", from_stream); option_import_marks(arg, from_stream, 0); } else if (skip_prefix(feature, "import-marks-if-exists=", &arg)) { + check_unsafe_feature("import-marks-if-exists", from_stream); option_import_marks(arg, from_stream, 1); } else if (skip_prefix(feature, "export-marks=", &arg)) { check_unsafe_feature(feature, from_stream); diff --git a/t/t9300-fast-import.sh b/t/t9300-fast-import.sh index ba5a35c32c..77104f9daa 100755 --- a/t/t9300-fast-import.sh +++ b/t/t9300-fast-import.sh @@ -2106,6 +2106,14 @@ test_expect_success 'R: abort on receiving feature after data command' ' test_must_fail git fast-import git.marks && + echo "feature import-marks=git.marks" >input && + test_must_fail git fast-import input && + test_must_fail git fast-import git.marks && >git2.marks && @@ -2114,7 +2122,7 @@ test_expect_success 'R: only one import-marks feature allowed per stream' ' feature import-marks=git2.marks EOF - test_must_fail git fast-import expect && - git fast-import --export-marks=io.marks <<-\EOF && + git fast-import --export-marks=io.marks \ + --allow-unsafe-features <<-\EOF && feature import-marks-if-exists=not_io.marks EOF test_cmp expect io.marks && @@ -2221,7 +2230,8 @@ test_expect_success 'R: feature import-marks-if-exists' ' echo ":1 $blob" >expect && echo ":2 $blob" >>expect && - git fast-import --export-marks=io.marks <<-\EOF && + git fast-import --export-marks=io.marks \ + --allow-unsafe-features <<-\EOF && feature import-marks-if-exists=io.marks blob mark :2 @@ -2234,7 +2244,8 @@ test_expect_success 'R: feature import-marks-if-exists' ' echo ":3 $blob" >>expect && git fast-import --import-marks=io.marks \ - --export-marks=io.marks <<-\EOF && + --export-marks=io.marks \ + --allow-unsafe-features <<-\EOF && feature import-marks-if-exists=not_io.marks blob mark :3 @@ -2247,7 +2258,8 @@ test_expect_success 'R: feature import-marks-if-exists' ' >expect && git fast-import --import-marks-if-exists=not_io.marks \ - --export-marks=io.marks <<-\EOF && + --export-marks=io.marks \ + --allow-unsafe-features <<-\EOF && feature import-marks-if-exists=io.marks EOF test_cmp expect io.marks