@matheusbsbs / matheus-bagliano / commits / e7d04ee147

Fork 0

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 <david.barr@cordelta.com> Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>

Jonathan Nieder committed Mar 26, 2011 at 00:15 UTC e7d04ee147dcbe6af1fa1d2147466696e2be31bc

2 files changed +37 -14

t/t9010-svn-fe.sh

+27

@@ -370,6 +370,33 @@ test_expect_failure 'change file mode but keep old content' '

370 test_cmp hello actual.target

371 '

372

373 +test_expect_success 'NUL in property value' '

374 + reinit_git &&

375 + echo "commit message" >expect.message &&

376 + {

377 + properties \

378 + unimportant "something with a NUL (Q)" \

379 + svn:log "commit message"&&

380 + echo PROPS-END

381 + } |

382 + q_to_nul >props &&

383 + {

384 + cat <<-\EOF &&

385 + SVN-fs-dump-format-version: 3

386 +

387 + Revision-number: 1

388 + EOF

389 + echo Prop-content-length: $(wc -c <props) &&

390 + echo Content-length: $(wc -c <props) &&

391 + echo &&

392 + cat props

393 + } >nulprop.dump &&

394 + test-svn-fe nulprop.dump >stream &&

395 + git fast-import <stream &&

396 + git diff-tree --always -s --format=%s HEAD >actual.message &&

397 + test_cmp expect.message actual.message

398 +'

399 +

400 test_expect_success 'change file mode and reiterate content' '

401 reinit_git &&

402 cat >expect <<-\EOF &&

vcs-svn/svndump.c

+10 -14

@@ -147,6 +147,7 @@ static void die_short_read(void)

147 static void read_props(void)

148 {

149 static struct strbuf key = STRBUF_INIT;

150 + static struct strbuf val = STRBUF_INIT;

151 const char *t;

152 /*

153 * NEEDSWORK: to support simple mode changes like

@@ -163,15 +164,15 @@ static void read_props(void)

164 uint32_t type_set = 0;

165 while ((t = buffer_read_line(&input)) && strcmp(t, "PROPS-END")) {

166 uint32_t len;

166 - const char *val;

167 const char type = t[0];

168 int ch;

169

170 if (!type || t[1] != ' ')

171 die("invalid property line: %s\n", t);

172 len = atoi(&t[2]);

173 - val = buffer_read_string(&input, len);

174 - if (!val || strlen(val) != len)

173 + strbuf_reset(&val);

174 + buffer_read_binary(&input, &val, len);

175 + if (val.len < len)

176 die_short_read();

177

178 /* Discard trailing newline. */

@@ -179,22 +180,17 @@ static void read_props(void)

180 if (ch == EOF)

181 die_short_read();

182 if (ch != '\n')

182 - die("invalid dump: expected newline after %s", val);

183 + die("invalid dump: expected newline after %s", val.buf);

184

185 switch (type) {

186 case 'K':

187 + strbuf_swap(&key, &val);

188 + continue;

189 case 'D':

187 - strbuf_reset(&key);

188 - if (val)

189 - strbuf_add(&key, val, len);

190 - if (type == 'K')

191 - continue;

192 - assert(type == 'D');

193 - val = NULL;

194 - len = 0;

195 - /* fall through */

190 + handle_property(&val, NULL, 0, &type_set);

191 + continue;

192 case 'V':

197 - handle_property(&key, val, len, &type_set);

193 + handle_property(&key, val.buf, len, &type_set);

194 strbuf_reset(&key);

195 continue;

196 default: