[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Subversion heap overflow
- To: bugtraq@xxxxxxxxxxxxxxxxx, full-disclosure@xxxxxxxxxxxxxxxxx
- Subject: Subversion heap overflow
- From: Matt Lewis <mattlewis@xxxxxxxxxx>
- Date: Fri, 7 Aug 2009 15:47:24 +0100
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. */
]]]