svn commit: r1714358 - in /subversion/trunk/subversion: libsvn_subr/string.c tests/libsvn_subr/string-test.c

[st...@apache.org]

Author: stefan2
Date: Sat Nov 14 20:16:29 2015
New Revision: 1714358

URL: http://svn.apache.org/viewvc?rev=1714358&view=rev
Log:
Fix the overflow / truncating handling handling of svn_stringbuf_remove
and svn_stringbuf_replace.  The API allows for deleted region to extend
beyond the current string; APR_SIZE_MAX in particular is a valid length
for the region to remove.

Note that all calls within our code use properly limited calls to these
functions, so they never may cause overflows.  3rd party callers might
be affected, though.

* subversion/libsvn_subr/string.c
  (svn_stringbuf_remove,
   svn_stringbuf_replace): Correct the removal length limiting code.

* subversion/tests/libsvn_subr/string-test.c
  (test_stringbuf_remove,
   test_stringbuf_replace): Add test cases for the fixed conditions.

Modified:
    subversion/trunk/subversion/libsvn_subr/string.c
    subversion/trunk/subversion/tests/libsvn_subr/string-test.c

Modified: subversion/trunk/subversion/libsvn_subr/string.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_subr/string.c?rev=1714358&r1=1714357&r2=1714358&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_subr/string.c (original)
+++ subversion/trunk/subversion/libsvn_subr/string.c Sat Nov 14 20:16:29 2015
@@ -677,7 +677,7 @@ svn_stringbuf_remove(svn_stringbuf_t *st
 {
   if (pos > str->len)
     pos = str->len;
-  if (pos + count > str->len)
+  if (count > str->len - pos)
     count = str->len - pos;
 
   memmove(str->data + pos, str->data + pos + count, str->len - pos - count + 1);
@@ -705,7 +705,7 @@ svn_stringbuf_replace(svn_stringbuf_t *s
 
   if (pos > str->len)
     pos = str->len;
-  if (pos + old_count > str->len)
+  if (old_count > str->len - pos)
     old_count = str->len - pos;
 
   if (old_count < new_count)

Modified: subversion/trunk/subversion/tests/libsvn_subr/string-test.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/tests/libsvn_subr/string-test.c?rev=1714358&r1=1714357&r2=1714358&view=diff
==============================================================================
--- subversion/trunk/subversion/tests/libsvn_subr/string-test.c (original)
+++ subversion/trunk/subversion/tests/libsvn_subr/string-test.c Sat Nov 14 20:16:29 2015
@@ -634,7 +634,15 @@ test_stringbuf_remove(apr_pool_t *pool)
   SVN_TEST_STRING_ASSERT(a->data, "stell");
 
   svn_stringbuf_remove(a, 1200, 393);
-  return expect_stringbuf_equal(a, "stell", pool);
+  SVN_ERR(expect_stringbuf_equal(a, "stell", pool));
+
+  svn_stringbuf_remove(a, APR_SIZE_MAX, 2);
+  SVN_ERR(expect_stringbuf_equal(a, "stell", pool));
+
+  svn_stringbuf_remove(a, 1, APR_SIZE_MAX);
+  SVN_ERR(expect_stringbuf_equal(a, "s", pool));
+
+  return SVN_NO_ERROR;
 }
 
 static svn_error_t *
@@ -672,6 +680,12 @@ test_stringbuf_replace(apr_pool_t *pool)
                     svn_stringbuf_ncreate("test hello\0-\0world!\0-\0!",
                                           23, pool)));
 
+  svn_stringbuf_replace(a, 1, APR_SIZE_MAX, "x", 1);
+  SVN_ERR(expect_stringbuf_equal(a, "tx", pool));
+
+  svn_stringbuf_replace(a, APR_SIZE_MAX, APR_SIZE_MAX, "y", 1);
+  SVN_ERR(expect_stringbuf_equal(a, "txy", pool));
+
   return SVN_NO_ERROR;
 }