lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [day] [month] [year] [list]
Message-ID: <ede346d80908070747p11a041a1w5f212a234b14599a@mail.gmail.com>
Date: Fri, 7 Aug 2009 15:47:24 +0100
From: Matt Lewis <mattlewis@...gle.com>
To: bugtraq@...urityfocus.com, full-disclosure@...ts.grok.org.uk
Subject: Subversion heap overflow

Subversion clients and servers, versions 1.6.0 - 1.6.3 and all
versions < 1.5.7, are vulnerable to several heap overflow problems
which may lead to remote code execution.  The official advisory
(mirrored at http://subversion.tigris.org/security/CVE-2009-2411-advisory.txt)
follows:


  Subversion clients and servers up to 1.6.3 (inclusive) have heap
  overflow issues in the parsing of binary deltas.

Summary:
========

  Subversion clients and servers have multiple heap overflow issues in
  the parsing of binary deltas.  This is related to an allocation
  vulnerability in the APR library used by Subversion.

  Clients with commit access to a vulnerable server can cause a remote
  heap overflow; servers can cause a heap overflow on vulnerable
  clients that try to do a checkout or update.

  This can lead to a DoS (an exploit has been tested) and to arbitrary
  code execution (no exploit tested, but the possibility is clear).

Known vulnerable:
=================

  Subversion clients and servers <= 1.5.6.
  Subversion clients and servers 1.6.0 through 1.6.3 (inclusive).

Known fixed:
============

  Subversion 1.6.4
  Subversion 1.5.7

  (Search for "Patch" below to see the patches from 1.6.3 -> 1.6.4 and
   1.5.6 -> 1.5.7.  Search for "Recommendations" to get URLs for the
   1.6.4 release and associated APR library patch.)

Details:
========

  The libsvn_delta library does not contain sufficient input validation
  of svndiff streams.  If a stream with large windows is processed,
  one of several integer overflows may lead to some boundary checks
  incorrectly passing, which in turn can lead to a heap overflow.

Severity:
=========

  A remote attacker with commit access to repository may be able to
  execute code on a Subversion server.  A malicious server may be able to
  execute code on a Subversion client.

Recommendations:
================

  We recommend all users to upgrade to Subversion 1.6.4.

  We recommend all users to upgrade to the latest versions of APR and
  APR-UTIL, or apply the CVE-2009-2412 patch appropriate to their APR
  installation from <http://www.apache.org/dist/apr/patches/>.

  New Subversion packages can be found at:
  http://subversion.tigris.org/project_packages.html

References:
===========

  CVE-2009-2411  (Subversion)
  CVE-2009-2412  (APR)

Reported by:
============

  Matt Lewis, Google.

Patches:
========

  This patch applies to Subversion 1.6.x (apply with patch -p0 < patchfile):

[[[
Index: subversion/libsvn_delta/svndiff.c
===================================================================
--- subversion/libsvn_delta/svndiff.c (revision 38519)
+++ subversion/libsvn_delta/svndiff.c (working copy)
@@ -60,10 +60,23 @@ struct encoder_baton {
   apr_pool_t *pool;
 };

+/* This is at least as big as the largest size of an integer that
+   encode_int can generate; it is sufficient for creating buffers for
+   it to write into.  This assumes that integers are at most 64 bits,
+   and so 10 bytes (with 7 bits of information each) are sufficient to
+   represent them. */
+#define MAX_ENCODED_INT_LEN 10
+/* This is at least as big as the largest size for a single instruction. */
+#define MAX_INSTRUCTION_LEN (2*MAX_ENCODED_INT_LEN+1)
+/* This is at least as big as the largest possible instructions
+   section: in theory, the instructions could be SVN_DELTA_WINDOW_SIZE
+   1-byte copy-from-source instructions (though this is very unlikely). */
+#define MAX_INSTRUCTION_SECTION_LEN (SVN_DELTA_WINDOW_SIZE*MAX_INSTRUCTION_LEN)

 /* Encode VAL into the buffer P using the variable-length svndiff
    integer format.  Return the incremented value of P after the
-   encoded bytes have been written.
+   encoded bytes have been written.  P must point to a buffer of size
+   at least MAX_ENCODED_INT_LEN.

    This encoding uses the high bit of each byte as a continuation bit
    and the other seven bits as data bits.  High-order data bits are
@@ -85,7 +98,7 @@ encode_int(char *p, svn_filesize_t val)
   svn_filesize_t v;
   unsigned char cont;

-  assert(val >= 0);
+  SVN_ERR_ASSERT_NO_RETURN(val >= 0);

   /* Figure out how many bytes we'll need.  */
   v = val >> 7;
@@ -96,6 +109,8 @@ encode_int(char *p, svn_filesize_t val)
       n++;
     }

+  SVN_ERR_ASSERT_NO_RETURN(n <= MAX_ENCODED_INT_LEN);
+
   /* Encode the remaining bytes; n is always the number of bytes
      coming after the one we're encoding.  */
   while (--n >= 0)
@@ -112,7 +127,7 @@ encode_int(char *p, svn_filesize_t val)
 static void
 append_encoded_int(svn_stringbuf_t *header, svn_filesize_t val)
 {
-  char buf[128], *p;
+  char buf[MAX_ENCODED_INT_LEN], *p;

   p = encode_int(buf, val);
   svn_stringbuf_appendbytes(header, buf, p - buf);
@@ -168,7 +183,7 @@ window_handler(svn_txdelta_window_t *window, void
   svn_stringbuf_t *i1 = svn_stringbuf_create("", pool);
   svn_stringbuf_t *header = svn_stringbuf_create("", pool);
   const svn_string_t *newdata;
-  char ibuf[128], *ip;
+  char ibuf[MAX_INSTRUCTION_LEN], *ip;
   const svn_txdelta_op_t *op;
   apr_size_t len;

@@ -346,6 +361,8 @@ decode_file_offset(svn_filesize_t *val,
                    const unsigned char *p,
                    const unsigned char *end)
 {
+  if (p + MAX_ENCODED_INT_LEN < end)
+    end = p + MAX_ENCODED_INT_LEN;
   /* Decode bytes until we're done.  */
   *val = 0;
   while (p < end)
@@ -365,6 +382,8 @@ decode_size(apr_size_t *val,
             const unsigned char *p,
             const unsigned char *end)
 {
+  if (p + MAX_ENCODED_INT_LEN < end)
+    end = p + MAX_ENCODED_INT_LEN;
   /* Decode bytes until we're done.  */
   *val = 0;
   while (p < end)
@@ -382,7 +401,7 @@ decode_size(apr_size_t *val,
    data is not compressed.  */

 static svn_error_t *
-zlib_decode(svn_stringbuf_t *in, svn_stringbuf_t *out)
+zlib_decode(svn_stringbuf_t *in, svn_stringbuf_t *out, apr_size_t limit)
 {
   apr_size_t len;
   char *oldplace = in->data;
@@ -390,6 +409,13 @@ static svn_error_t *
   /* First thing in the string is the original length.  */
   in->data = (char *)decode_size(&len, (unsigned char *)in->data,
                                  (unsigned char *)in->data+in->len);
+  if (in->data == NULL)
+    return svn_error_create(SVN_ERR_SVNDIFF_INVALID_COMPRESSED_DATA, NULL,
+                            _("Decompression of svndiff data failed:
no size"));
+  if (len > limit)
+    return svn_error_create(SVN_ERR_SVNDIFF_INVALID_COMPRESSED_DATA, NULL,
+                            _("Decompression of svndiff data failed: "
+                              "size too large"));
   /* We need to subtract the size of the encoded original length off the
    *      still remaining input length.  */
   in->len -= (in->data - oldplace);
@@ -487,10 +513,10 @@ count_and_verify_instructions(int *ninst,
         return svn_error_createf
           (SVN_ERR_SVNDIFF_INVALID_OPS, NULL,
            _("Invalid diff stream: insn %d cannot be decoded"), n);
-      else if (op.length <= 0)
+      else if (op.length == 0)
         return svn_error_createf
           (SVN_ERR_SVNDIFF_INVALID_OPS, NULL,
-           _("Invalid diff stream: insn %d has non-positive length"), n);
+           _("Invalid diff stream: insn %d has length zero"), n);
       else if (op.length > tview_len - tpos)
         return svn_error_createf
           (SVN_ERR_SVNDIFF_INVALID_OPS, NULL,
@@ -499,7 +525,8 @@ count_and_verify_instructions(int *ninst,
       switch (op.action_code)
         {
         case svn_txdelta_source:
-          if (op.length > sview_len - op.offset)
+          if (op.length > sview_len - op.offset ||
+              op.offset > sview_len)
             return svn_error_createf
               (SVN_ERR_SVNDIFF_INVALID_OPS, NULL,
                _("Invalid diff stream: "
@@ -565,11 +592,11 @@ decode_window(svn_txdelta_window_t *window, svn_fi

       instin = svn_stringbuf_ncreate((const char *)data, insend - data, pool);
       instout = svn_stringbuf_create("", pool);
-      SVN_ERR(zlib_decode(instin, instout));
+      SVN_ERR(zlib_decode(instin, instout, MAX_INSTRUCTION_SECTION_LEN));

       ndin = svn_stringbuf_ncreate((const char *)insend, newlen, pool);
       ndout = svn_stringbuf_create("", pool);
-      SVN_ERR(zlib_decode(ndin, ndout));
+      SVN_ERR(zlib_decode(ndin, ndout, SVN_DELTA_WINDOW_SIZE));

       newlen = ndout->len;
       data = (unsigned char *)instout->data;
@@ -685,6 +712,14 @@ write_handler(void *baton,
       if (p == NULL)
         return SVN_NO_ERROR;

+      if (tview_len > SVN_DELTA_WINDOW_SIZE ||
+          sview_len > SVN_DELTA_WINDOW_SIZE ||
+          /* for svndiff1, newlen includes the original length */
+          newlen > SVN_DELTA_WINDOW_SIZE + MAX_ENCODED_INT_LEN ||
+          inslen > MAX_INSTRUCTION_SECTION_LEN)
+        return svn_error_create(SVN_ERR_SVNDIFF_CORRUPT_WINDOW, NULL,
+                                _("Svndiff contains a too-large window"));
+
       /* Check for integer overflow.  */
       if (sview_offset < 0 || inslen + newlen < inslen
           || sview_len + tview_len < sview_len
@@ -841,6 +876,14 @@ read_window_header(svn_stream_t *stream, svn_files
   SVN_ERR(read_one_size(inslen, stream));
   SVN_ERR(read_one_size(newlen, stream));

+  if (*tview_len > SVN_DELTA_WINDOW_SIZE ||
+      *sview_len > SVN_DELTA_WINDOW_SIZE ||
+      /* for svndiff1, newlen includes the original length */
+      *newlen > SVN_DELTA_WINDOW_SIZE + MAX_ENCODED_INT_LEN ||
+      *inslen > MAX_INSTRUCTION_SECTION_LEN)
+    return svn_error_create(SVN_ERR_SVNDIFF_CORRUPT_WINDOW, NULL,
+                            _("Svndiff contains a too-large window"));
+
   /* Check for integer overflow.  */
   if (*sview_offset < 0 || *inslen + *newlen < *inslen
       || *sview_len + *tview_len < *sview_len
Index: subversion/libsvn_delta/text_delta.c
===================================================================
--- subversion/libsvn_delta/text_delta.c  (revision 38519)
+++ subversion/libsvn_delta/text_delta.c  (working copy)
@@ -548,7 +548,7 @@ svn_txdelta_target_push(svn_txdelta_window_handler
 /* Functions for applying deltas.  */

 /* Ensure that BUF has enough space for VIEW_LEN bytes.  */
-static APR_INLINE void
+static APR_INLINE svn_error_t *
 size_buffer(char **buf, apr_size_t *buf_size,
             apr_size_t view_len, apr_pool_t *pool)
 {
@@ -557,8 +557,11 @@ size_buffer(char **buf, apr_size_t *buf_size,
       *buf_size *= 2;
       if (*buf_size < view_len)
         *buf_size = view_len;
+      SVN_ERR_ASSERT(APR_ALIGN_DEFAULT(*buf_size) >= *buf_size);
       *buf = apr_palloc(pool, *buf_size);
     }
+
+  return SVN_NO_ERROR;
 }


@@ -659,7 +662,7 @@ apply_window(svn_txdelta_window_t *window, void *b
                          >= ab->sbuf_offset + ab->sbuf_len)));

   /* Make sure there's enough room in the target buffer.  */
-  size_buffer(&ab->tbuf, &ab->tbuf_size, window->tview_len, ab->pool);
+  SVN_ERR(size_buffer(&ab->tbuf, &ab->tbuf_size, window->tview_len, ab->pool));

   /* Prepare the source buffer for reading from the input stream.  */
   if (window->sview_offset != ab->sbuf_offset
@@ -668,7 +671,8 @@ apply_window(svn_txdelta_window_t *window, void *b
       char *old_sbuf = ab->sbuf;

       /* Make sure there's enough room.  */
-      size_buffer(&ab->sbuf, &ab->sbuf_size, window->sview_len, ab->pool);
+      SVN_ERR(size_buffer(&ab->sbuf, &ab->sbuf_size, window->sview_len,
+              ab->pool));

       /* If the existing view overlaps with the new view, copy the
        * overlap to the beginning of the new buffer.  */
]]]


  This patch applies to Subversion 1.5.x:

[[[
Index: subversion/libsvn_delta/svndiff.c
===================================================================
--- subversion/libsvn_delta/svndiff.c (revision 38498)
+++ subversion/libsvn_delta/svndiff.c (working copy)
@@ -55,10 +55,23 @@ struct encoder_baton {
   apr_pool_t *pool;
 };

+/* This is at least as big as the largest size of an integer that
+   encode_int can generate; it is sufficient for creating buffers for
+   it to write into.  This assumes that integers are at most 64 bits,
+   and so 10 bytes (with 7 bits of information each) are sufficient to
+   represent them. */
+#define MAX_ENCODED_INT_LEN 10
+/* This is at least as big as the largest size for a single instruction. */
+#define MAX_INSTRUCTION_LEN (2*MAX_ENCODED_INT_LEN+1)
+/* This is at least as big as the largest possible instructions
+   section: in theory, the instructions could be SVN_DELTA_WINDOW_SIZE
+   1-byte copy-from-source instructions (though this is very unlikely). */
+#define MAX_INSTRUCTION_SECTION_LEN (SVN_DELTA_WINDOW_SIZE*MAX_INSTRUCTION_LEN)

 /* Encode VAL into the buffer P using the variable-length svndiff
    integer format.  Return the incremented value of P after the
-   encoded bytes have been written.
+   encoded bytes have been written.  P must point to a buffer of size
+   at least MAX_ENCODED_INT_LEN.

    This encoding uses the high bit of each byte as a continuation bit
    and the other seven bits as data bits.  High-order data bits are
@@ -91,6 +104,8 @@ encode_int(char *p, svn_filesize_t val)
       n++;
     }

+  assert(n <= MAX_ENCODED_INT_LEN);
+
   /* Encode the remaining bytes; n is always the number of bytes
      coming after the one we're encoding.  */
   while (--n >= 0)
@@ -107,7 +122,7 @@ encode_int(char *p, svn_filesize_t val)
 static void
 append_encoded_int(svn_stringbuf_t *header, svn_filesize_t val)
 {
-  char buf[128], *p;
+  char buf[MAX_ENCODED_INT_LEN], *p;

   p = encode_int(buf, val);
   svn_stringbuf_appendbytes(header, buf, p - buf);
@@ -163,7 +178,7 @@ window_handler(svn_txdelta_window_t *window, void
   svn_stringbuf_t *i1 = svn_stringbuf_create("", pool);
   svn_stringbuf_t *header = svn_stringbuf_create("", pool);
   const svn_string_t *newdata;
-  char ibuf[128], *ip;
+  char ibuf[MAX_INSTRUCTION_LEN], *ip;
   const svn_txdelta_op_t *op;
   apr_size_t len;

@@ -341,6 +356,8 @@ decode_file_offset(svn_filesize_t *val,
                    const unsigned char *p,
                    const unsigned char *end)
 {
+  if (p + MAX_ENCODED_INT_LEN < end)
+    end = p + MAX_ENCODED_INT_LEN;
   /* Decode bytes until we're done.  */
   *val = 0;
   while (p < end)
@@ -360,6 +377,8 @@ decode_size(apr_size_t *val,
             const unsigned char *p,
             const unsigned char *end)
 {
+  if (p + MAX_ENCODED_INT_LEN < end)
+    end = p + MAX_ENCODED_INT_LEN;
   /* Decode bytes until we're done.  */
   *val = 0;
   while (p < end)
@@ -377,7 +396,7 @@ decode_size(apr_size_t *val,
    data is not compressed.  */

 static svn_error_t *
-zlib_decode(svn_stringbuf_t *in, svn_stringbuf_t *out)
+zlib_decode(svn_stringbuf_t *in, svn_stringbuf_t *out, apr_size_t limit)
 {
   apr_size_t len;
   char *oldplace = in->data;
@@ -385,6 +404,13 @@ static svn_error_t *
   /* First thing in the string is the original length.  */
   in->data = (char *)decode_size(&len, (unsigned char *)in->data,
                                  (unsigned char *)in->data+in->len);
+  if (in->data == NULL)
+    return svn_error_create(SVN_ERR_SVNDIFF_INVALID_COMPRESSED_DATA, NULL,
+                            _("Decompression of svndiff data failed:
no size"));
+  if (len > limit)
+    return svn_error_create(SVN_ERR_SVNDIFF_INVALID_COMPRESSED_DATA, NULL,
+                            _("Decompression of svndiff data failed: "
+                              "size too large"));
   /* We need to subtract the size of the encoded original length off the
    *      still remaining input length.  */
   in->len -= (in->data - oldplace);
@@ -482,10 +508,10 @@ count_and_verify_instructions(int *ninst,
         return svn_error_createf
           (SVN_ERR_SVNDIFF_INVALID_OPS, NULL,
            _("Invalid diff stream: insn %d cannot be decoded"), n);
-      else if (op.length <= 0)
+      else if (op.length == 0)
         return svn_error_createf
           (SVN_ERR_SVNDIFF_INVALID_OPS, NULL,
-           _("Invalid diff stream: insn %d has non-positive length"), n);
+           _("Invalid diff stream: insn %d has length zero"), n);
       else if (op.length > tview_len - tpos)
         return svn_error_createf
           (SVN_ERR_SVNDIFF_INVALID_OPS, NULL,
@@ -494,7 +520,8 @@ count_and_verify_instructions(int *ninst,
       switch (op.action_code)
         {
         case svn_txdelta_source:
-          if (op.length > sview_len - op.offset)
+          if (op.length > sview_len - op.offset ||
+              op.offset > sview_len)
             return svn_error_createf
               (SVN_ERR_SVNDIFF_INVALID_OPS, NULL,
                _("Invalid diff stream: "
@@ -560,11 +587,11 @@ decode_window(svn_txdelta_window_t *window, svn_fi

       instin = svn_stringbuf_ncreate((const char *)data, insend - data, pool);
       instout = svn_stringbuf_create("", pool);
-      SVN_ERR(zlib_decode(instin, instout));
+      SVN_ERR(zlib_decode(instin, instout, MAX_INSTRUCTION_SECTION_LEN));

       ndin = svn_stringbuf_ncreate((const char *)insend, newlen, pool);
       ndout = svn_stringbuf_create("", pool);
-      SVN_ERR(zlib_decode(ndin, ndout));
+      SVN_ERR(zlib_decode(ndin, ndout, SVN_DELTA_WINDOW_SIZE));

       newlen = ndout->len;
       data = (unsigned char *)instout->data;
@@ -680,6 +707,14 @@ write_handler(void *baton,
       if (p == NULL)
         return SVN_NO_ERROR;

+      if (tview_len > SVN_DELTA_WINDOW_SIZE ||
+          sview_len > SVN_DELTA_WINDOW_SIZE ||
+          /* for svndiff1, newlen includes the original length */
+          newlen > SVN_DELTA_WINDOW_SIZE + MAX_ENCODED_INT_LEN ||
+          inslen > MAX_INSTRUCTION_SECTION_LEN)
+        return svn_error_create(SVN_ERR_SVNDIFF_CORRUPT_WINDOW, NULL,
+                                _("Svndiff contains a too-large window"));
+
       /* Check for integer overflow.  */
       if (sview_offset < 0 || inslen + newlen < inslen
           || sview_len + tview_len < sview_len
@@ -836,6 +871,14 @@ read_window_header(svn_stream_t *stream, svn_files
   SVN_ERR(read_one_size(inslen, stream));
   SVN_ERR(read_one_size(newlen, stream));

+  if (*tview_len > SVN_DELTA_WINDOW_SIZE ||
+      *sview_len > SVN_DELTA_WINDOW_SIZE ||
+      /* for svndiff1, newlen includes the original length */
+      *newlen > SVN_DELTA_WINDOW_SIZE + MAX_ENCODED_INT_LEN ||
+      *inslen > MAX_INSTRUCTION_SECTION_LEN)
+    return svn_error_create(SVN_ERR_SVNDIFF_CORRUPT_WINDOW, NULL,
+                            _("Svndiff contains a too-large window"));
+
   /* Check for integer overflow.  */
   if (*sview_offset < 0 || *inslen + *newlen < *inslen
       || *sview_len + *tview_len < *sview_len
Index: subversion/libsvn_delta/text_delta.c
===================================================================
--- subversion/libsvn_delta/text_delta.c  (revision 38498)
+++ subversion/libsvn_delta/text_delta.c  (working copy)
@@ -498,7 +498,7 @@ svn_txdelta_target_push(svn_txdelta_window_handler
 /* Functions for applying deltas.  */

 /* Ensure that BUF has enough space for VIEW_LEN bytes.  */
-static APR_INLINE void
+static APR_INLINE svn_error_t *
 size_buffer(char **buf, apr_size_t *buf_size,
             apr_size_t view_len, apr_pool_t *pool)
 {
@@ -507,8 +507,13 @@ size_buffer(char **buf, apr_size_t *buf_size,
       *buf_size *= 2;
       if (*buf_size < view_len)
         *buf_size = view_len;
+      if (APR_ALIGN_DEFAULT(*buf_size) < *buf_size)
+        return svn_error_create(SVN_ERR_SVNDIFF_INVALID_OPS, NULL,
+                                "Diff stream resulted in invalid
buffer size.");
       *buf = apr_palloc(pool, *buf_size);
     }
+
+  return SVN_NO_ERROR;
 }


@@ -609,7 +614,7 @@ apply_window(svn_txdelta_window_t *window, void *b
                  >= ab->sbuf_offset + ab->sbuf_len)));

   /* Make sure there's enough room in the target buffer.  */
-  size_buffer(&ab->tbuf, &ab->tbuf_size, window->tview_len, ab->pool);
+  SVN_ERR(size_buffer(&ab->tbuf, &ab->tbuf_size, window->tview_len, ab->pool));

   /* Prepare the source buffer for reading from the input stream.  */
   if (window->sview_offset != ab->sbuf_offset
@@ -618,7 +623,8 @@ apply_window(svn_txdelta_window_t *window, void *b
       char *old_sbuf = ab->sbuf;

       /* Make sure there's enough room.  */
-      size_buffer(&ab->sbuf, &ab->sbuf_size, window->sview_len, ab->pool);
+      SVN_ERR(size_buffer(&ab->sbuf, &ab->sbuf_size, window->sview_len,
+              ab->pool));

       /* If the existing view overlaps with the new view, copy the
        * overlap to the beginning of the new buffer.  */
]]]

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ