Merge branch 'jk/fast-import-unsafe'
The `--export-marks` option of `git fast-import` is exposed also via the in-stream command `feature export-marks=...` and it allows overwriting arbitrary paths. This topic branch prevents the in-stream version, to prevent arbitrary file accesses by `git fast-import` streams coming from untrusted sources (e.g. in remote helpers that are based on `git fast-import`). This fixes CVE-2019-1348. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
This commit is contained in:
commit
a7b1ad3b05
@ -50,6 +50,21 @@ OPTIONS
|
|||||||
memory used by fast-import during this run. Showing this output
|
memory used by fast-import during this run. Showing this output
|
||||||
is currently the default, but can be disabled with --quiet.
|
is currently the default, but can be disabled with --quiet.
|
||||||
|
|
||||||
|
--allow-unsafe-features::
|
||||||
|
Many command-line options can be provided as part of the
|
||||||
|
fast-import stream itself by using the `feature` or `option`
|
||||||
|
commands. However, some of these options are unsafe (e.g.,
|
||||||
|
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 `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
|
||||||
|
remote-helpers that use the `import` capability, as they are
|
||||||
|
already trusted to run their own code.
|
||||||
|
|
||||||
Options for Frontends
|
Options for Frontends
|
||||||
~~~~~~~~~~~~~~~~~~~~~
|
~~~~~~~~~~~~~~~~~~~~~
|
||||||
|
|
||||||
|
@ -366,6 +366,7 @@ static uintmax_t next_mark;
|
|||||||
static struct strbuf new_data = STRBUF_INIT;
|
static struct strbuf new_data = STRBUF_INIT;
|
||||||
static int seen_data_command;
|
static int seen_data_command;
|
||||||
static int require_explicit_termination;
|
static int require_explicit_termination;
|
||||||
|
static int allow_unsafe_features;
|
||||||
|
|
||||||
/* Signal handling */
|
/* Signal handling */
|
||||||
static volatile sig_atomic_t checkpoint_requested;
|
static volatile sig_atomic_t checkpoint_requested;
|
||||||
@ -1861,6 +1862,12 @@ static void dump_marks(void)
|
|||||||
if (!export_marks_file || (import_marks_file && !import_marks_file_done))
|
if (!export_marks_file || (import_marks_file && !import_marks_file_done))
|
||||||
return;
|
return;
|
||||||
|
|
||||||
|
if (safe_create_leading_directories_const(export_marks_file)) {
|
||||||
|
failure |= error_errno("unable to create leading directories of %s",
|
||||||
|
export_marks_file);
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
|
||||||
if (hold_lock_file_for_update(&mark_lock, export_marks_file, 0) < 0) {
|
if (hold_lock_file_for_update(&mark_lock, export_marks_file, 0) < 0) {
|
||||||
failure |= error_errno("Unable to write marks file %s",
|
failure |= error_errno("Unable to write marks file %s",
|
||||||
export_marks_file);
|
export_marks_file);
|
||||||
@ -3228,7 +3235,6 @@ static void option_import_marks(const char *marks,
|
|||||||
}
|
}
|
||||||
|
|
||||||
import_marks_file = make_fast_import_path(marks);
|
import_marks_file = make_fast_import_path(marks);
|
||||||
safe_create_leading_directories_const(import_marks_file);
|
|
||||||
import_marks_file_from_stream = from_stream;
|
import_marks_file_from_stream = from_stream;
|
||||||
import_marks_file_ignore_missing = ignore_missing;
|
import_marks_file_ignore_missing = ignore_missing;
|
||||||
}
|
}
|
||||||
@ -3269,7 +3275,6 @@ static void option_active_branches(const char *branches)
|
|||||||
static void option_export_marks(const char *marks)
|
static void option_export_marks(const char *marks)
|
||||||
{
|
{
|
||||||
export_marks_file = make_fast_import_path(marks);
|
export_marks_file = make_fast_import_path(marks);
|
||||||
safe_create_leading_directories_const(export_marks_file);
|
|
||||||
}
|
}
|
||||||
|
|
||||||
static void option_cat_blob_fd(const char *fd)
|
static void option_cat_blob_fd(const char *fd)
|
||||||
@ -3312,10 +3317,12 @@ static int parse_one_option(const char *option)
|
|||||||
option_active_branches(option);
|
option_active_branches(option);
|
||||||
} else if (skip_prefix(option, "export-pack-edges=", &option)) {
|
} else if (skip_prefix(option, "export-pack-edges=", &option)) {
|
||||||
option_export_pack_edges(option);
|
option_export_pack_edges(option);
|
||||||
} else if (starts_with(option, "quiet")) {
|
} else if (!strcmp(option, "quiet")) {
|
||||||
show_stats = 0;
|
show_stats = 0;
|
||||||
} else if (starts_with(option, "stats")) {
|
} else if (!strcmp(option, "stats")) {
|
||||||
show_stats = 1;
|
show_stats = 1;
|
||||||
|
} else if (!strcmp(option, "allow-unsafe-features")) {
|
||||||
|
; /* already handled during early option parsing */
|
||||||
} else {
|
} else {
|
||||||
return 0;
|
return 0;
|
||||||
}
|
}
|
||||||
@ -3323,6 +3330,13 @@ static int parse_one_option(const char *option)
|
|||||||
return 1;
|
return 1;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
static void check_unsafe_feature(const char *feature, int from_stream)
|
||||||
|
{
|
||||||
|
if (from_stream && !allow_unsafe_features)
|
||||||
|
die(_("feature '%s' forbidden in input without --allow-unsafe-features"),
|
||||||
|
feature);
|
||||||
|
}
|
||||||
|
|
||||||
static int parse_one_feature(const char *feature, int from_stream)
|
static int parse_one_feature(const char *feature, int from_stream)
|
||||||
{
|
{
|
||||||
const char *arg;
|
const char *arg;
|
||||||
@ -3330,10 +3344,13 @@ static int parse_one_feature(const char *feature, int from_stream)
|
|||||||
if (skip_prefix(feature, "date-format=", &arg)) {
|
if (skip_prefix(feature, "date-format=", &arg)) {
|
||||||
option_date_format(arg);
|
option_date_format(arg);
|
||||||
} else if (skip_prefix(feature, "import-marks=", &arg)) {
|
} else if (skip_prefix(feature, "import-marks=", &arg)) {
|
||||||
|
check_unsafe_feature("import-marks", from_stream);
|
||||||
option_import_marks(arg, from_stream, 0);
|
option_import_marks(arg, from_stream, 0);
|
||||||
} else if (skip_prefix(feature, "import-marks-if-exists=", &arg)) {
|
} 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);
|
option_import_marks(arg, from_stream, 1);
|
||||||
} else if (skip_prefix(feature, "export-marks=", &arg)) {
|
} else if (skip_prefix(feature, "export-marks=", &arg)) {
|
||||||
|
check_unsafe_feature(feature, from_stream);
|
||||||
option_export_marks(arg);
|
option_export_marks(arg);
|
||||||
} else if (!strcmp(feature, "get-mark")) {
|
} else if (!strcmp(feature, "get-mark")) {
|
||||||
; /* Don't die - this feature is supported */
|
; /* Don't die - this feature is supported */
|
||||||
@ -3460,6 +3477,20 @@ int cmd_main(int argc, const char **argv)
|
|||||||
avail_tree_table = xcalloc(avail_tree_table_sz, sizeof(struct avail_tree_content*));
|
avail_tree_table = xcalloc(avail_tree_table_sz, sizeof(struct avail_tree_content*));
|
||||||
marks = pool_calloc(1, sizeof(struct mark_set));
|
marks = pool_calloc(1, sizeof(struct mark_set));
|
||||||
|
|
||||||
|
/*
|
||||||
|
* We don't parse most options until after we've seen the set of
|
||||||
|
* "feature" lines at the start of the stream (which allows the command
|
||||||
|
* line to override stream data). But we must do an early parse of any
|
||||||
|
* command-line options that impact how we interpret the feature lines.
|
||||||
|
*/
|
||||||
|
for (i = 1; i < argc; i++) {
|
||||||
|
const char *arg = argv[i];
|
||||||
|
if (*arg != '-' || !strcmp(arg, "--"))
|
||||||
|
break;
|
||||||
|
if (!strcmp(arg, "--allow-unsafe-features"))
|
||||||
|
allow_unsafe_features = 1;
|
||||||
|
}
|
||||||
|
|
||||||
global_argc = argc;
|
global_argc = argc;
|
||||||
global_argv = argv;
|
global_argv = argv;
|
||||||
|
|
||||||
|
@ -2106,12 +2106,27 @@ test_expect_success 'R: abort on receiving feature after data command' '
|
|||||||
test_must_fail git fast-import <input
|
test_must_fail git fast-import <input
|
||||||
'
|
'
|
||||||
|
|
||||||
|
test_expect_success 'R: import-marks features forbidden by default' '
|
||||||
|
>git.marks &&
|
||||||
|
echo "feature import-marks=git.marks" >input &&
|
||||||
|
test_must_fail git fast-import <input &&
|
||||||
|
echo "feature import-marks-if-exists=git.marks" >input &&
|
||||||
|
test_must_fail git fast-import <input
|
||||||
|
'
|
||||||
|
|
||||||
test_expect_success 'R: only one import-marks feature allowed per stream' '
|
test_expect_success 'R: only one import-marks feature allowed per stream' '
|
||||||
|
>git.marks &&
|
||||||
|
>git2.marks &&
|
||||||
cat >input <<-EOF &&
|
cat >input <<-EOF &&
|
||||||
feature import-marks=git.marks
|
feature import-marks=git.marks
|
||||||
feature import-marks=git2.marks
|
feature import-marks=git2.marks
|
||||||
EOF
|
EOF
|
||||||
|
|
||||||
|
test_must_fail git fast-import --allow-unsafe-features <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_must_fail git fast-import <input
|
||||||
'
|
'
|
||||||
|
|
||||||
@ -2125,19 +2140,29 @@ test_expect_success 'R: export-marks feature results in a marks file being creat
|
|||||||
|
|
||||||
EOF
|
EOF
|
||||||
|
|
||||||
cat input | git fast-import &&
|
git fast-import --allow-unsafe-features <input &&
|
||||||
grep :1 git.marks
|
grep :1 git.marks
|
||||||
'
|
'
|
||||||
|
|
||||||
test_expect_success 'R: export-marks options can be overridden by commandline options' '
|
test_expect_success 'R: export-marks options can be overridden by commandline options' '
|
||||||
cat input | git fast-import --export-marks=other.marks &&
|
cat >input <<-\EOF &&
|
||||||
grep :1 other.marks
|
feature export-marks=feature-sub/git.marks
|
||||||
|
blob
|
||||||
|
mark :1
|
||||||
|
data 3
|
||||||
|
hi
|
||||||
|
|
||||||
|
EOF
|
||||||
|
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
|
||||||
'
|
'
|
||||||
|
|
||||||
test_expect_success 'R: catch typo in marks file name' '
|
test_expect_success 'R: catch typo in marks file name' '
|
||||||
test_must_fail git fast-import --import-marks=nonexistent.marks </dev/null &&
|
test_must_fail git fast-import --import-marks=nonexistent.marks </dev/null &&
|
||||||
echo "feature import-marks=nonexistent.marks" |
|
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' '
|
test_expect_success 'R: import and output marks can be the same file' '
|
||||||
@ -2193,7 +2218,8 @@ test_expect_success 'R: feature import-marks-if-exists' '
|
|||||||
rm -f io.marks &&
|
rm -f io.marks &&
|
||||||
>expect &&
|
>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
|
feature import-marks-if-exists=not_io.marks
|
||||||
EOF
|
EOF
|
||||||
test_cmp expect io.marks &&
|
test_cmp expect io.marks &&
|
||||||
@ -2204,7 +2230,8 @@ test_expect_success 'R: feature import-marks-if-exists' '
|
|||||||
echo ":1 $blob" >expect &&
|
echo ":1 $blob" >expect &&
|
||||||
echo ":2 $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
|
feature import-marks-if-exists=io.marks
|
||||||
blob
|
blob
|
||||||
mark :2
|
mark :2
|
||||||
@ -2217,7 +2244,8 @@ test_expect_success 'R: feature import-marks-if-exists' '
|
|||||||
echo ":3 $blob" >>expect &&
|
echo ":3 $blob" >>expect &&
|
||||||
|
|
||||||
git fast-import --import-marks=io.marks \
|
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
|
feature import-marks-if-exists=not_io.marks
|
||||||
blob
|
blob
|
||||||
mark :3
|
mark :3
|
||||||
@ -2230,7 +2258,8 @@ test_expect_success 'R: feature import-marks-if-exists' '
|
|||||||
>expect &&
|
>expect &&
|
||||||
|
|
||||||
git fast-import --import-marks-if-exists=not_io.marks \
|
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
|
feature import-marks-if-exists=io.marks
|
||||||
EOF
|
EOF
|
||||||
test_cmp expect io.marks
|
test_cmp expect io.marks
|
||||||
@ -2242,7 +2271,7 @@ test_expect_success 'R: import to output marks works without any content' '
|
|||||||
feature export-marks=marks.new
|
feature export-marks=marks.new
|
||||||
EOF
|
EOF
|
||||||
|
|
||||||
cat input | git fast-import &&
|
git fast-import --allow-unsafe-features <input &&
|
||||||
test_cmp marks.out marks.new
|
test_cmp marks.out marks.new
|
||||||
'
|
'
|
||||||
|
|
||||||
@ -2252,7 +2281,7 @@ test_expect_success 'R: import marks prefers commandline marks file over the str
|
|||||||
feature export-marks=marks.new
|
feature export-marks=marks.new
|
||||||
EOF
|
EOF
|
||||||
|
|
||||||
cat input | git fast-import --import-marks=marks.out &&
|
git fast-import --import-marks=marks.out --allow-unsafe-features <input &&
|
||||||
test_cmp marks.out marks.new
|
test_cmp marks.out marks.new
|
||||||
'
|
'
|
||||||
|
|
||||||
@ -2265,7 +2294,8 @@ test_expect_success 'R: multiple --import-marks= should be honoured' '
|
|||||||
|
|
||||||
head -n2 marks.out > one.marks &&
|
head -n2 marks.out > one.marks &&
|
||||||
tail -n +3 marks.out > two.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
|
test_cmp marks.out combined.marks
|
||||||
'
|
'
|
||||||
|
|
||||||
@ -2278,7 +2308,7 @@ test_expect_success 'R: feature relative-marks should be honoured' '
|
|||||||
|
|
||||||
mkdir -p .git/info/fast-import/ &&
|
mkdir -p .git/info/fast-import/ &&
|
||||||
cp marks.new .git/info/fast-import/relative.in &&
|
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
|
test_cmp marks.new .git/info/fast-import/relative.out
|
||||||
'
|
'
|
||||||
|
|
||||||
@ -2290,7 +2320,7 @@ test_expect_success 'R: feature no-relative-marks should be honoured' '
|
|||||||
feature export-marks=non-relative.out
|
feature export-marks=non-relative.out
|
||||||
EOF
|
EOF
|
||||||
|
|
||||||
git fast-import <input &&
|
git fast-import --allow-unsafe-features <input &&
|
||||||
test_cmp marks.new non-relative.out
|
test_cmp marks.new non-relative.out
|
||||||
'
|
'
|
||||||
|
|
||||||
@ -2560,7 +2590,7 @@ test_expect_success 'R: quiet option results in no stats being output' '
|
|||||||
|
|
||||||
EOF
|
EOF
|
||||||
|
|
||||||
cat input | git fast-import 2> output &&
|
git fast-import 2>output <input &&
|
||||||
test_must_be_empty output
|
test_must_be_empty output
|
||||||
'
|
'
|
||||||
|
|
||||||
|
@ -431,6 +431,7 @@ static int get_importer(struct transport *transport, struct child_process *fasti
|
|||||||
child_process_init(fastimport);
|
child_process_init(fastimport);
|
||||||
fastimport->in = helper->out;
|
fastimport->in = helper->out;
|
||||||
argv_array_push(&fastimport->args, "fast-import");
|
argv_array_push(&fastimport->args, "fast-import");
|
||||||
|
argv_array_push(&fastimport->args, "--allow-unsafe-features");
|
||||||
argv_array_push(&fastimport->args, debug ? "--stats" : "--quiet");
|
argv_array_push(&fastimport->args, debug ? "--stats" : "--quiet");
|
||||||
|
|
||||||
if (data->bidi_import) {
|
if (data->bidi_import) {
|
||||||
|
Loading…
Reference in New Issue
Block a user