So, now this patch seems ready to go in. It does three things, all
related to each other pretty much:
- Prevent going below root with '..'.
- Correctly UTF-8 translate root paths.
- Makes root path absolute before using.
Currently 'svn_repos_open' isn't expecting an UTF-8 path, but that is
another bug to be fixed. And 'svnadmin' is already giving it UTF-8
paths anyway.
Do take a peek at the INT_ERR() macro there - it's stolen almost
verbatim from 'svnadmin', but I don't know if that's still the way to
go or if it should be handled differently.
If this seems fine, I'll commit it.
-- Naked
Log message:
Prevent using '..' in URLs to go below given root path in
svnserve. Also fix UTF-8 translation for root path. Also make given
root path absolute before using since apr_proc_detach changes the
current directory.
* subversion/svnserve/serve.c: Include svn_utf.h.
(find_repos): Use apr_filepath_merge with APR_FILEPATH_SECUREROOT to
combine root path with client path.
(find_repos): Do UTF-8 translation before and after apr_filepath_merge.
* subversion/svnserve/main.c: Include svn_utf.h.
(main): Translate root path to UTF-8.
(main): Make root path absolute before detaching.
* subversion/libsvn_ra_svn/todo: Removed note in security about '..'.
Patch:
Index: subversion/libsvn_ra_svn/todo
===================================================================
--- subversion/libsvn_ra_svn/todo (revision 4141)
+++ subversion/libsvn_ra_svn/todo (working copy)
@@ -63,9 +63,6 @@
There is no way to disable anonymous authentication. Fixing that
will probably wait for a server configuration file.
-The now the client URL can escape from the logical root area using
-".." path elements. The server should check for that.
-
Port number
-----------
Index: subversion/svnserve/serve.c
===================================================================
--- subversion/svnserve/serve.c (revision 4141)
+++ subversion/svnserve/serve.c (working copy)
@@ -25,6 +25,7 @@
#include <apr_strings.h>
#include <apr_network_io.h>
#include <apr_user.h>
+#include <apr_file_info.h>
#include <svn_types.h>
#include <svn_string.h>
@@ -35,6 +36,7 @@
#include <svn_repos.h>
#include <svn_path.h>
#include <svn_time.h>
+#include <svn_utf.h>
#include "server.h"
@@ -841,7 +843,10 @@
const char **fs_path, apr_pool_t *pool)
{
svn_error_t *err;
+ apr_status_t apr_err;
const char *client_path, *full_path, *candidate;
+ const char *client_path_native, *root_native;
+ char *buffer;
/* Decode any escaped characters in the URL. */
url = svn_path_uri_decode(url, pool);
@@ -855,10 +860,25 @@
client_path = strchr(url + 6, '/');
client_path = (client_path == NULL) ? "" : client_path + 1;
+ SVN_ERR(svn_utf_cstring_from_utf8(&client_path_native,
+ svn_path_canonicalize(client_path, pool),
+ pool));
+
+ SVN_ERR(svn_utf_cstring_from_utf8(&root_native,
+ svn_path_canonicalize(root, pool),
+ pool));
+
/* Join the server-configured root with the client path. */
- full_path = svn_path_join(svn_path_canonicalize(root, pool),
- svn_path_canonicalize(client_path, pool),
- pool);
+ apr_err = apr_filepath_merge(&buffer, root_native, client_path_native,
+ APR_FILEPATH_SECUREROOT, pool);
+
+ if(apr_err)
+ return svn_error_create(SVN_ERR_BAD_FILENAME, 0, NULL,
+ "Couldn't determine repository path.");
+
+ SVN_ERR(svn_utf_cstring_to_utf8(&full_path,
+ svn_path_canonicalize (buffer, pool),
+ NULL, pool));
/* Search for a repository in the full path. */
candidate = full_path;
Index: subversion/svnserve/main.c
===================================================================
--- subversion/svnserve/main.c (revision 4141)
+++ subversion/svnserve/main.c (working copy)
@@ -33,6 +33,8 @@
#include "svn_pools.h"
#include "svn_error.h"
#include "svn_ra_svn.h"
+#include "svn_utf.h"
+#include "svn_path.h"
#include "server.h"
@@ -49,6 +51,14 @@
/* Nothing to do; we just need to interrupt the accept(). */
}
+#define INT_ERR(expr) \
+ do { \
+ svn_error_t *svnserve_err__temp = (expr); \
+ if (svnserve_err__temp) { \
+ svn_handle_error (svnserve_err__temp, stderr, FALSE); \
+ return EXIT_FAILURE; } \
+ } while (0)
+
int main(int argc, const char *const *argv)
{
svn_boolean_t listen_once = FALSE, daemon_mode = FALSE, tunnel_mode = FALSE;
@@ -94,7 +104,8 @@
break;
case 'r':
- root = arg;
+ INT_ERR(svn_utf_cstring_to_utf8(&root, arg, NULL, pool));
+ INT_ERR(svn_path_get_absolute(&root, root, pool));
break;
}
}
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Tue Dec 17 18:24:12 2002