vcs-svn: avoid hangs from corrupt deltas

A corrupt Subversion-format delta can request reads past the end of
the preimage.  Set sliding_view::max_off so such corruption is caught
when it appears rather than blocking in an impossible-to-fulfill
read() when input is coming from a socket or pipe.

Inspired-by: Ramkumar Ramachandra <artagnon@gmail.com>
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>

diff --git a/t/t9010-svn-fe.sh b/t/t9010-svn-fe.sh
index f24f004fd5cc2ad7c68969d289e23bc7dd8744a6..b7eed2489fa169aa1c6e5c98f59a0349ba8118fa 100755 (executable)
--- a/t/t9010-svn-fe.sh
+++ b/t/t9010-svn-fe.sh
@@ -18,12 +18,13 @@ reinit_git () {
 
 try_dump () {
        input=$1 &&
 
 try_dump () {
        input=$1 &&

-       maybe_fail=${2:+test_$2} &&
+       maybe_fail_svnfe=${2:+test_$2} &&
+       maybe_fail_fi=${3:+test_$3} &&

 
        {
 
        {

-               $maybe_fail test-svn-fe "$input" >stream 3<backflow &
+               $maybe_fail_svnfe test-svn-fe "$input" >stream 3<backflow &

        } &&
        } &&

-       git fast-import --cat-blob-fd=3 <stream 3>backflow &&
+       $maybe_fail_fi git fast-import --cat-blob-fd=3 <stream 3>backflow &&

        wait $!
 }
 
        wait $!
 }
 


@@ -1047,6 +1048,39 @@ test_expect_success PIPE 'deltas need not consume the whole preimage' '
        test_cmp expect.3 actual.3
 '
 
        test_cmp expect.3 actual.3
 '
 

+test_expect_success PIPE 'no hang for delta trying to read past end of preimage' '
+       reinit_git &&
+       {
+               # COPY 1
+               printf "SVNQ%b%b" "Q\001\001\002Q" "\001Q" |
+               q_to_nul
+       } >greedy.delta &&
+       {
+               cat <<-\EOF &&
+               SVN-fs-dump-format-version: 3
+
+               Revision-number: 1
+               Prop-content-length: 10
+               Content-length: 10
+
+               PROPS-END
+
+               Node-path: bootstrap
+               Node-kind: file
+               Node-action: add
+               Text-delta: true
+               Prop-content-length: 10
+               EOF
+               echo Text-content-length: $(wc -c <greedy.delta) &&
+               echo Content-length: $((10 + $(wc -c <greedy.delta))) &&
+               echo &&
+               echo PROPS-END &&
+               cat greedy.delta &&
+               echo
+       } >greedydelta.dump &&
+       try_dump greedydelta.dump must_fail might_fail
+'
+

 test_expect_success 'set up svn repo' '
        svnconf=$PWD/svnconf &&
        mkdir -p "$svnconf" &&
 test_expect_success 'set up svn repo' '
        svnconf=$PWD/svnconf &&
        mkdir -p "$svnconf" &&

diff --git a/vcs-svn/fast_export.c b/vcs-svn/fast_export.c
index 96a75d51d16c75d562758f62be2ca31e6eaf0f39..97f5fdf48985d408a4944ad678fb04a5ad9c6157 100644 (file)
--- a/vcs-svn/fast_export.c
+++ b/vcs-svn/fast_export.c
@@ -198,8 +198,7 @@ static long apply_delta(off_t len, struct line_buffer *input,
                        const char *old_data, uint32_t old_mode)
 {
        long ret;
                        const char *old_data, uint32_t old_mode)
 {
        long ret;

-       off_t preimage_len = 0;
-       struct sliding_view preimage = SLIDING_VIEW_INIT(&report_buffer, -1);
+       struct sliding_view preimage = SLIDING_VIEW_INIT(&report_buffer, 0);

        FILE *out;
 
        if (init_postimage() || !(out = buffer_tmpfile_rewind(&postimage)))
        FILE *out;
 
        if (init_postimage() || !(out = buffer_tmpfile_rewind(&postimage)))


@@ -211,19 +210,23 @@ static long apply_delta(off_t len, struct line_buffer *input,
                printf("cat-blob %s\n", old_data);
                fflush(stdout);
                response = get_response_line();
                printf("cat-blob %s\n", old_data);
                fflush(stdout);
                response = get_response_line();

-               if (parse_cat_response_line(response, &preimage_len))
+               if (parse_cat_response_line(response, &preimage.max_off))

                        die("invalid cat-blob response: %s", response);
                        die("invalid cat-blob response: %s", response);

+               check_preimage_overflow(preimage.max_off, 1);

        }
        if (old_mode == REPO_MODE_LNK) {
                strbuf_addstr(&preimage.buf, "link ");
        }
        if (old_mode == REPO_MODE_LNK) {
                strbuf_addstr(&preimage.buf, "link ");

-               check_preimage_overflow(preimage_len, strlen("link "));
-               preimage_len += strlen("link ");
+               check_preimage_overflow(preimage.max_off, strlen("link "));
+               preimage.max_off += strlen("link ");
+               check_preimage_overflow(preimage.max_off, 1);

        }
        if (svndiff0_apply(input, len, &preimage, out))
                die("cannot apply delta");
        if (old_data) {
                /* Read the remainder of preimage and trailing newline. */
        }
        if (svndiff0_apply(input, len, &preimage, out))
                die("cannot apply delta");
        if (old_data) {
                /* Read the remainder of preimage and trailing newline. */

-               if (move_window(&preimage, preimage_len, 1))
+               assert(!signed_add_overflows(preimage.max_off, 1));
+               preimage.max_off++;     /* room for newline */
+               if (move_window(&preimage, preimage.max_off - 1, 1))

                        die("cannot seek to end of input");
                if (preimage.buf.buf[0] != '\n')
                        die("missing newline after cat-blob response");
                        die("cannot seek to end of input");
                if (preimage.buf.buf[0] != '\n')
                        die("missing newline after cat-blob response");