From e7d04ee147dcbe6af1fa1d2147466696e2be31bc Mon Sep 17 00:00:00 2001 From: Jonathan Nieder Date: Sat, 26 Mar 2011 00:15:10 -0500 Subject: [PATCH] vcs-svn: make reading of properties binary-safe svn-fe errors out on revision 59151 of the ASF repository: fatal: invalid dump: unexpected end of file The proximate cause is a property with an embedded NUL character. Previously such anomalies were ignored but commit c9d1c8ba (2010-12-28) introduced a check strlen(val) == len to avoid reading uninitialized data when a property list ends early and unfortunately this test does not distinguish between "foo" followed by EOF and the string "foo\0bar\0baz". Fix it by using buffer_read_binary to read to a strbuf and checking the actual length read. Most consumers of properties still use C-style strings, so in practice an author or log message with embedded NULs will be truncated, but a least this way svn-fe won't error out (fixing the regression). Reported-by: David Barr Signed-off-by: Jonathan Nieder --- t/t9010-svn-fe.sh | 27 +++++++++++++++++++++++++++ vcs-svn/svndump.c | 24 ++++++++++-------------- 2 files changed, 37 insertions(+), 14 deletions(-) diff --git a/t/t9010-svn-fe.sh b/t/t9010-svn-fe.sh index 5a6a4b9b7..5e2b12308 100755 --- a/t/t9010-svn-fe.sh +++ b/t/t9010-svn-fe.sh @@ -370,6 +370,33 @@ test_expect_failure 'change file mode but keep old content' ' test_cmp hello actual.target ' +test_expect_success 'NUL in property value' ' + reinit_git && + echo "commit message" >expect.message && + { + properties \ + unimportant "something with a NUL (Q)" \ + svn:log "commit message"&& + echo PROPS-END + } | + q_to_nul >props && + { + cat <<-\EOF && + SVN-fs-dump-format-version: 3 + + Revision-number: 1 + EOF + echo Prop-content-length: $(wc -c nulprop.dump && + test-svn-fe nulprop.dump >stream && + git fast-import actual.message && + test_cmp expect.message actual.message +' + test_expect_success 'change file mode and reiterate content' ' reinit_git && cat >expect <<-\EOF && diff --git a/vcs-svn/svndump.c b/vcs-svn/svndump.c index ea5b128e4..c00f03117 100644 --- a/vcs-svn/svndump.c +++ b/vcs-svn/svndump.c @@ -147,6 +147,7 @@ static void die_short_read(void) static void read_props(void) { static struct strbuf key = STRBUF_INIT; + static struct strbuf val = STRBUF_INIT; const char *t; /* * NEEDSWORK: to support simple mode changes like @@ -163,15 +164,15 @@ static void read_props(void) uint32_t type_set = 0; while ((t = buffer_read_line(&input)) && strcmp(t, "PROPS-END")) { uint32_t len; - const char *val; const char type = t[0]; int ch; if (!type || t[1] != ' ') die("invalid property line: %s\n", t); len = atoi(&t[2]); - val = buffer_read_string(&input, len); - if (!val || strlen(val) != len) + strbuf_reset(&val); + buffer_read_binary(&input, &val, len); + if (val.len < len) die_short_read(); /* Discard trailing newline. */ @@ -179,22 +180,17 @@ static void read_props(void) if (ch == EOF) die_short_read(); if (ch != '\n') - die("invalid dump: expected newline after %s", val); + die("invalid dump: expected newline after %s", val.buf); switch (type) { case 'K': + strbuf_swap(&key, &val); + continue; case 'D': - strbuf_reset(&key); - if (val) - strbuf_add(&key, val, len); - if (type == 'K') - continue; - assert(type == 'D'); - val = NULL; - len = 0; - /* fall through */ + handle_property(&val, NULL, 0, &type_set); + continue; case 'V': - handle_property(&key, val, len, &type_set); + handle_property(&key, val.buf, len, &type_set); strbuf_reset(&key); continue; default: -- 2.30.2