Skip to main content

Browse the archive

Revision 6d8684161ee9c03bed5cb69ae76dfdddb85a0003 authored by Johannes Schindelin on 13 September 2019, 14:32:43 UTC, committed by Johannes Schindelin on 05 December 2019, 14:36:51 UTC 
mingw: fix quoting of arguments
 
We need to be careful to follow proper quoting rules. For example, if an
argument contains spaces, we have to quote them. Double-quotes need to
be escaped. Backslashes need to be escaped, but only if they are
followed by a double-quote character.

We need to be _extra_ careful to consider the case where an argument
ends in a backslash _and_ needs to be quoted: in this case, we append a
double-quote character, i.e. the backslash now has to be escaped!

The current code, however, fails to recognize that, and therefore can
turn an argument that ends in a single backslash into a quoted argument
that now ends in an escaped double-quote character. This allows
subsequent command-line parameters to be split and part of them being
mistaken for command-line options, e.g. through a maliciously-crafted
submodule URL during a recursive clone.

Technically, we would not need to quote _all_ arguments which end in a
backslash _unless_ the argument needs to be quoted anyway. For example,
`test\` would not need to be quoted, while `test \` would need to be.

To keep the code simple, however, and therefore easier to reason about
and ensure its correctness, we now _always_ quote an argument that ends
in a backslash.

This addresses CVE-2019-1350.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
1 parent a8dee3c
Raw File
t3902-quoted.sh 
#!/bin/sh
#
# Copyright (c) 2006 Junio C Hamano
#

test_description='quoted output'

. ./test-lib.sh

FN='濱野'
GN='純'
HT='	'
DQ='"'

test_have_prereq MINGW ||
echo foo 2>/dev/null > "Name and an${HT}HT"
if ! test -f "Name and an${HT}HT"
then
	# FAT/NTFS does not allow tabs in filenames
	skip_all='Your filesystem does not allow tabs in filenames'
	test_done
fi

for_each_name () {
	for name in \
	    Name "Name and a${LF}LF" "Name and an${HT}HT" "Name${DQ}" \
	    "$FN$HT$GN" "$FN$LF$GN" "$FN $GN" "$FN$GN" "$FN$DQ$GN" \
	    "With SP in it" "$FN/file"
	do
		eval "$1"
	done
}

test_expect_success 'setup' '

	mkdir "$FN" &&
	for_each_name "echo initial >\"\$name\"" &&
	git add . &&
	git commit -q -m Initial &&

	for_each_name "echo second >\"\$name\"" &&
	git commit -a -m Second &&

	for_each_name "echo modified >\"\$name\""

'

test_expect_success 'setup expected files' '
cat >expect.quoted <<\EOF &&
Name
"Name and a\nLF"
"Name and an\tHT"
"Name\""
With SP in it
"\346\277\261\351\207\216\t\347\264\224"
"\346\277\261\351\207\216\n\347\264\224"
"\346\277\261\351\207\216 \347\264\224"
"\346\277\261\351\207\216\"\347\264\224"
"\346\277\261\351\207\216/file"
"\346\277\261\351\207\216\347\264\224"
EOF

cat >expect.raw <<\EOF
Name
"Name and a\nLF"
"Name and an\tHT"
"Name\""
With SP in it
"濱野\t純"
"濱野\n純"
濱野 純
"濱野\"純"
濱野/file
濱野純
EOF
'

test_expect_success 'check fully quoted output from ls-files' '

	git ls-files >current && test_cmp expect.quoted current

'

test_expect_success 'check fully quoted output from diff-files' '

	git diff --name-only >current &&
	test_cmp expect.quoted current

'

test_expect_success 'check fully quoted output from diff-index' '

	git diff --name-only HEAD >current &&
	test_cmp expect.quoted current

'

test_expect_success 'check fully quoted output from diff-tree' '

	git diff --name-only HEAD^ HEAD >current &&
	test_cmp expect.quoted current

'

test_expect_success 'check fully quoted output from ls-tree' '

	git ls-tree --name-only -r HEAD >current &&
	test_cmp expect.quoted current

'

test_expect_success 'setting core.quotepath' '

	git config --bool core.quotepath false

'

test_expect_success 'check fully quoted output from ls-files' '

	git ls-files >current && test_cmp expect.raw current

'

test_expect_success 'check fully quoted output from diff-files' '

	git diff --name-only >current &&
	test_cmp expect.raw current

'

test_expect_success 'check fully quoted output from diff-index' '

	git diff --name-only HEAD >current &&
	test_cmp expect.raw current

'

test_expect_success 'check fully quoted output from diff-tree' '

	git diff --name-only HEAD^ HEAD >current &&
	test_cmp expect.raw current

'

test_expect_success 'check fully quoted output from ls-tree' '

	git ls-tree --name-only -r HEAD >current &&
	test_cmp expect.raw current

'

test_done
back to top