[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

[PATCH] Issue #1947 Submission #3 svn_path_uri_decode may copy garbage and overrun buffer when given partial % escape

From: Klaus Rennecke <kre_at_tigris.org>
Date: 2004-07-07 22:36:07 CEST

Branko Čibej wrote:
> Klaus Rennecke wrote:
>> Index: D:/kre/workspace/svn/subversion/libsvn_subr/path.c
>> ===================================================================
>> [...]
>> +#include <ctype.h> /* for svn_path_uri_decode() */
>> [...]
> This should be "#include <apr_lib.h>" and "apr_isxdigit". Otherwise, +1.
> We should make a similar change in svn_path_is_uri_safe.

That is fixed with this new patch. I had a look around and noticed that
isxdigit is used in some places. For instance in fs_fs.c. But I cannot
judge if that's any problem.

> Hm. I hope noone ever tries to port Subversion to an EBCDIC platform,
> because not one of these checks will work correctly there... :-\

I'm now directly comparing to ASCII values, so this should not be an
issue anymore. Keeps things simple as well. :-)

Should I try to squeeze the new patches onto the issue tracker?

[[[
Check that the two characters following the % escape are valid hex
digits. This serves to check for premature end of input as well.

Fixes Issue #1947 svn_path_uri_decode may copy garbage and overrun
buffer when given partial % escape.

* subversion/libsvn_subr/path.c
   (svn_path_uri_decode): Check syntax of % escape.

* subversion/tests/libsvn_subr/path-test.c
   (test_uri_decode): New test function.
   (test_funcs): Added test_uri_decode.
]]]

/Klaus

Index: D:/kre/workspace/svn/subversion/libsvn_subr/path.c
===================================================================
--- D:/kre/workspace/svn/subversion/libsvn_subr/path.c (revision 10168)
+++ D:/kre/workspace/svn/subversion/libsvn_subr/path.c (working copy)
@@ -43,6 +43,12 @@
    the OS! */
 #define SVN_PATH_IS_PLATFORM_EMPTY(s,n) ((n) == 1 && (s)[0] == '.')
 
+/* TRUE if c is a valid hex digit in ASCII, as defined per RFC 2396
+ section 2. Used in svn_path_uri_decode. */
+#define SVN_PATH_IS_URLXDIGIT(c) (((c) >= 48 && (c) <= 57) \
+ || ((c) >= 65 && (c) <= 70) \
+ || ((c) >= 97 && (c) <= 102))
+
 
 const char *
 svn_path_internal_style (const char *path, apr_pool_t *pool)
@@ -892,7 +898,9 @@
            * RFC 2396, section 3.3 */
           c = ' ';
         }
- else if (c == '%')
+ else if (c == '%'
+ && SVN_PATH_IS_URLXDIGIT (path[i + 1])
+ && SVN_PATH_IS_URLXDIGIT (path[i + 2]))
         {
           char digitz[3];
           digitz[0] = path[++i];

Index: D:/kre/workspace/svn/subversion/tests/libsvn_subr/path-test.c
===================================================================
--- D:/kre/workspace/svn/subversion/tests/libsvn_subr/path-test.c (revision 10170)
+++ D:/kre/workspace/svn/subversion/tests/libsvn_subr/path-test.c (working copy)
@@ -292,6 +292,45 @@
 
 
 static svn_error_t *
+test_uri_decode (const char **msg,
+ svn_boolean_t msg_only,
+ apr_pool_t *pool)
+{
+ int i;
+
+ const char *paths[3][2] = {
+ { "http://c.r.a/s%\0008me",
+ "http://c.r.a/s%"},
+ { "http://c.r.a/s%6\000me",
+ "http://c.r.a/s%6" },
+ { "http://c.r.a/s%68me",
+ "http://c.r.a/shme" },
+ };
+
+ *msg = "test svn_path_uri_decode with invalid escape";
+
+ if (msg_only)
+ return SVN_NO_ERROR;
+
+ for (i = 0; i < 3; i++)
+ {
+ const char *de_path;
+
+ /* URI-decode the path, and verify the results. */
+ de_path = svn_path_uri_decode (paths[i][0], pool);
+ if (strcmp (de_path, paths[i][1]))
+ {
+ return svn_error_createf
+ (SVN_ERR_TEST_FAILED, NULL,
+ "svn_path_uri_decode ('%s') returned '%s' instead of '%s'",
+ paths[i][0], de_path, paths[i][1]);
+ }
+ }
+ return SVN_NO_ERROR;
+}
+
+
+static svn_error_t *
 test_join (const char **msg,
            svn_boolean_t msg_only,
            apr_pool_t *pool)
@@ -605,6 +644,7 @@
     SVN_TEST_PASS (test_is_url),
     SVN_TEST_PASS (test_is_uri_safe),
     SVN_TEST_PASS (test_uri_encode),
+ SVN_TEST_PASS (test_uri_decode),
     SVN_TEST_PASS (test_join),
     SVN_TEST_PASS (test_basename),
     SVN_TEST_PASS (test_decompose),

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Wed Jul 7 22:36:28 2004

This is an archived mail posted to the Subversion Dev mailing list.