Fix invalid read in quote_c_style_counted

This function did not work on strings that were not NUL-terminated. It
reads through a length-bounded string, searching for characters in need of
quoting. After we find one, we output the quoted character, then advance
our pointer to find the next one. However, we never decremented the
length, meaning we ended up looking at whatever random junk was stored
after the string.

This bug was not found by the existing tests because most code paths feed
a NUL-terminated string. The notable exception is a directory name being
fed by ls-tree.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/quote.c b/quote.c
index acb6bf929f3e5807139fa7852f739e6ff5a50722..fc93435727db3b0634c390965258200c61d8b59b 100644 (file)
--- a/quote.c
+++ b/quote.c
@@ -213,7 +213,7 @@ static size_t quote_c_style_counted(const char *name, ssize_t maxlen,
                int ch;
 
                len = next_quote_pos(p, maxlen);
-               if (len == maxlen || !p[len])
+               if (len == maxlen || (maxlen < 0 && !p[len]))
                        break;
 
                if (!no_dq && p == name)
@@ -223,6 +223,8 @@ static size_t quote_c_style_counted(const char *name, ssize_t maxlen,
                EMIT('\\');
                p += len;
                ch = (unsigned char)*p++;
+               if (maxlen >= 0)
+                       maxlen -= len + 1;
                if (sq_lookup[ch] >= ' ') {
                        EMIT(sq_lookup[ch]);
                } else {

diff --git a/t/t3902-quoted.sh b/t/t3902-quoted.sh
index 58680524258037eb0c17f7e2929d58ed1c16b574..14da45fe5a19f7436d9748f934c59b8a431d30dc 100755 (executable)
--- a/t/t3902-quoted.sh
+++ b/t/t3902-quoted.sh
@@ -25,7 +25,7 @@ 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"
+           "With SP in it" "caractère spécial/file"
        do
                eval "$1"
        done
@@ -33,6 +33,7 @@ for_each_name () {
 
 test_expect_success setup '
 
+       mkdir "caractère spécial" &&
        for_each_name "echo initial >\"\$name\""
        git add . &&
        git commit -q -m Initial &&
@@ -50,6 +51,7 @@ Name
 "Name and an\tHT"
 "Name\""
 With SP in it
+"caract\303\250re sp\303\251cial/file"
 "\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"
@@ -63,6 +65,7 @@ Name
 "Name and an\tHT"
 "Name\""
 With SP in it
+caractère spécial/file
 "濱野\t純"
 "濱野\n純"
 濱野 純
@@ -97,6 +100,13 @@ test_expect_success 'check fully quoted output from diff-tree' '
 
 '
 
+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
@@ -130,4 +140,11 @@ test_expect_success 'check fully quoted output from diff-tree' '
 
 '
 
+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