Hi,

I've noticed that the test 'test_string_matching' introduced in r1505336 [1]
violates the contracts of the svn_cstring__match_length and
svn_cstring__reverse_match_length functions:

[[[
    Return the number of bytes before A and B that don't differ.  If no
    difference can be found in the first MAX_LEN characters,  MAX_LEN will
    be returned.  Please note that A-MAX_LEN and B-MAX_LEN must both be
    valid addresses.
]]]

Currently this test sets the MAX_LEN value to the maximum of the string
lengths.  This violates the "valid addresses" part of the contract and results
in undefined behavior due to the out-of-bounds memory access.

On my 64-bit Ubuntu machine this test fails when compiled with the string
pooling optimization (gcc -O1 / -O2 / -O3):

[[[
    PASS:  string-test 27: check deletion from svn_stringbuf_t
    PASS:  string-test 28: check replacement in svn_stringbuf_t
    PASS:  string-test 29: test string similarity scores
    svn_tests: E200006: assertion 'rmatch_len == test->rmatch_len' failed at
      subversion/tests/libsvn_subr/string-test.c:823
    FAIL:  string-test 30: test string matching
]]]

I've attached a patch that fixes this issue.

[1]: https://svn.apache.org/viewvc?view=revision&revision=r1505336


Thanks and regards,
Evgeny Kotkov
Index: subversion/tests/libsvn_subr/string-test.c
===================================================================
--- subversion/tests/libsvn_subr/string-test.c  (revision 1507762)
+++ subversion/tests/libsvn_subr/string-test.c  (working copy)
@@ -812,7 +812,7 @@ test_string_matching(apr_pool_t *pool)
     {
       apr_size_t a_len = strlen(test->a);
       apr_size_t b_len = strlen(test->b);
-      apr_size_t max_match = MAX(a_len, b_len);
+      apr_size_t max_match = MIN(a_len, b_len);
       apr_size_t match_len
         = svn_cstring__match_length(test->a, test->b, max_match);
       apr_size_t rmatch_len
Fix the out-of-bounds memory access in test_string_matching_test.  Follow-up
to r1505336.

* subversion/tests/libsvn_subr/string-test.c
  (test_string_matching): Correctly initialize the 'max_match' value to avoid
    out-of-bounds memory access.

Patch by: Evgeny Kotkov <evgeny.kotkov{_AT_}visualsvn.com>

Reply via email to