From 953982c839d91366b9591f00a5d1e5abb431c9bd Mon Sep 17 00:00:00 2001 From: Daniel Sahlberg Date: Sun, 8 Dec 2024 23:49:59 +0000 Subject: [PATCH] Commit the patches for CVE-2024-46901 TODO: Pls help me update the log message git-svn-id: https://svn.apache.org/repos/asf/subversion/trunk@1922383 13f79535-47bb-0310-9956-ffa450edef68 --- .../include/private/svn_repos_private.h | 8 +++ subversion/libsvn_repos/commit.c | 3 +- subversion/libsvn_repos/repos.c | 10 +++ subversion/mod_dav_svn/lock.c | 7 +++ subversion/mod_dav_svn/repos.c | 30 +++++++++ subversion/tests/cmdline/mod_dav_svn_tests.py | 62 +++++++++++++++++++ 6 files changed, 118 insertions(+), 2 deletions(-) diff --git a/subversion/include/private/svn_repos_private.h b/subversion/include/private/svn_repos_private.h index 5faaab6485..f80100ac56 100644 --- a/subversion/include/private/svn_repos_private.h +++ b/subversion/include/private/svn_repos_private.h @@ -390,6 +390,14 @@ svn_repos__get_dump_editor(const svn_delta_editor_t **editor, const char *update_anchor_relpath, apr_pool_t *pool); +/* Validate that the given PATH is a valid pathname that can be stored in + * a Subversion repository, according to the name constraints used by the + * svn_repos_* layer. + */ +svn_error_t * +svn_repos__validate_new_path(const char *path, + apr_pool_t *scratch_pool); + #ifdef __cplusplus } #endif /* __cplusplus */ diff --git a/subversion/libsvn_repos/commit.c b/subversion/libsvn_repos/commit.c index dca8887a93..486dedd092 100644 --- a/subversion/libsvn_repos/commit.c +++ b/subversion/libsvn_repos/commit.c @@ -308,8 +308,7 @@ add_file_or_directory(const char *path, svn_boolean_t was_copied = FALSE; const char *full_path, *canonicalized_path; - /* Reject paths which contain control characters (related to issue #4340). */ - SVN_ERR(svn_path_check_valid(path, pool)); + SVN_ERR(svn_repos__validate_new_path(path, pool)); SVN_ERR(svn_relpath_canonicalize_safe(&canonicalized_path, NULL, path, pool, pool)); diff --git a/subversion/libsvn_repos/repos.c b/subversion/libsvn_repos/repos.c index 2c2267674e..1c9d8dc660 100644 --- a/subversion/libsvn_repos/repos.c +++ b/subversion/libsvn_repos/repos.c @@ -2092,3 +2092,13 @@ svn_repos__fs_type(const char **fs_type, svn_dirent_join(repos_path, SVN_REPOS__DB_DIR, pool), pool); } + +svn_error_t * +svn_repos__validate_new_path(const char *path, + apr_pool_t *scratch_pool) +{ + /* Reject paths which contain control characters (related to issue #4340). */ + SVN_ERR(svn_path_check_valid(path, scratch_pool)); + + return SVN_NO_ERROR; +} diff --git a/subversion/mod_dav_svn/lock.c b/subversion/mod_dav_svn/lock.c index 7e9c94b64d..d2a6aa9021 100644 --- a/subversion/mod_dav_svn/lock.c +++ b/subversion/mod_dav_svn/lock.c @@ -36,6 +36,7 @@ #include "svn_pools.h" #include "svn_props.h" #include "private/svn_log.h" +#include "private/svn_repos_private.h" #include "dav_svn.h" @@ -717,6 +718,12 @@ append_locks(dav_lockdb *lockdb, /* Commit a 0-byte file: */ + if ((serr = svn_repos__validate_new_path(resource->info->repos_path, + resource->pool))) + return dav_svn__convert_err(serr, HTTP_BAD_REQUEST, + "Request specifies an invalid path.", + resource->pool); + if ((serr = dav_svn__get_youngest_rev(&rev, repos, resource->pool))) return dav_svn__convert_err(serr, HTTP_INTERNAL_SERVER_ERROR, "Could not determine youngest revision", diff --git a/subversion/mod_dav_svn/repos.c b/subversion/mod_dav_svn/repos.c index 4eec268f9a..d39b6c7d14 100644 --- a/subversion/mod_dav_svn/repos.c +++ b/subversion/mod_dav_svn/repos.c @@ -2928,6 +2928,16 @@ open_stream(const dav_resource *resource, if (kind == svn_node_none) /* No existing file. */ { + serr = svn_repos__validate_new_path(resource->info->repos_path, + resource->pool); + + if (serr != NULL) + { + return dav_svn__convert_err(serr, HTTP_BAD_REQUEST, + "Request specifies an invalid path.", + resource->pool); + } + serr = svn_fs_make_file(resource->info->root.root, resource->info->repos_path, resource->pool); @@ -4120,6 +4130,14 @@ create_collection(dav_resource *resource) return err; } + if ((serr = svn_repos__validate_new_path(resource->info->repos_path, + resource->pool)) != NULL) + { + return dav_svn__convert_err(serr, HTTP_BAD_REQUEST, + "Request specifies an invalid path.", + resource->pool); + } + if ((serr = svn_fs_make_dir(resource->info->root.root, resource->info->repos_path, resource->pool)) != NULL) @@ -4194,6 +4212,12 @@ copy_resource(const dav_resource *src, return err; } + serr = svn_repos__validate_new_path(dst->info->repos_path, dst->pool); + if (serr) + return dav_svn__convert_err(serr, HTTP_BAD_REQUEST, + "Request specifies an invalid path.", + dst->pool); + src_repos_path = svn_repos_path(src->info->repos->repos, src->pool); dst_repos_path = svn_repos_path(dst->info->repos->repos, dst->pool); @@ -4430,6 +4454,12 @@ move_resource(dav_resource *src, if (err) return err; + serr = svn_repos__validate_new_path(dst->info->repos_path, dst->pool); + if (serr) + return dav_svn__convert_err(serr, HTTP_BAD_REQUEST, + "Request specifies an invalid path.", + dst->pool); + /* Copy the src to the dst. */ serr = svn_fs_copy(src->info->root.root, /* the root object of src rev*/ src->info->repos_path, /* the relative path of src */ diff --git a/subversion/tests/cmdline/mod_dav_svn_tests.py b/subversion/tests/cmdline/mod_dav_svn_tests.py index 9628fa9fc0..2489f30310 100755 --- a/subversion/tests/cmdline/mod_dav_svn_tests.py +++ b/subversion/tests/cmdline/mod_dav_svn_tests.py @@ -686,6 +686,67 @@ def last_modified_header(sbox): raise svntest.Failure('Unexpected Last-Modified header: %s' % last_modified) r.read() +@SkipUnless(svntest.main.is_ra_type_dav) +def create_name_with_control_chars(sbox): + "test creating items with control chars in names" + + sbox.build(create_wc=False) + + h = svntest.main.create_http_connection(sbox.repo_url) + + # POST /repos/!svn/me + # Create a new transaction. + req_body = ( + '(create-txn-with-props ' + '(svn:txn-client-compat-version 6 1.14.4 ' + 'svn:txn-user-agent 45 SVN/1.14.4 (x86-microsoft-windows) serf/1.3.9 ' + 'svn:log 0 ))' + ) + headers = { + 'Authorization': 'Basic ' + base64.b64encode(b'jconstant:rayjandom').decode(), + 'Content-Type': 'application/vnd.svn-skel', + } + h.request('POST', sbox.repo_url + '/!svn/me', req_body, headers) + r = h.getresponse() + if r.status != httplib.CREATED: + raise svntest.Failure('Unexpected status: %d %s' % (r.status, r.reason)) + txn_name = r.getheader('SVN-Txn-Name') + r.read() + + # MKCOL /repos/!svn/txn/TXN_NAME/tab%09name + # Must fail with a 400 Bad Request. + headers = { + 'Authorization': 'Basic ' + base64.b64encode(b'jconstant:rayjandom').decode(), + } + h.request('MKCOL', sbox.repo_url + '/!svn/txr/' + txn_name + '/tab%09name', None, headers) + r = h.getresponse() + if r.status != httplib.BAD_REQUEST: + raise svntest.Failure('Unexpected status: %d %s' % (r.status, r.reason)) + r.read() + + # PUT /repos/!svn/txn/TXN_NAME/tab%09name + # Must fail with a 400 Bad Request. + headers = { + 'Authorization': 'Basic ' + base64.b64encode(b'jconstant:rayjandom').decode(), + } + h.request('PUT', sbox.repo_url + '/!svn/txr/' + txn_name + '/tab%09name', None, headers) + r = h.getresponse() + if r.status != httplib.BAD_REQUEST: + raise svntest.Failure('Unexpected status: %d %s' % (r.status, r.reason)) + r.read() + + # COPY /repos/!svn/rvr/1/iota -> /repos/!svn/txn/TXN_NAME/tab%09name + # Must fail with a 400 Bad Request. + headers = { + 'Authorization': 'Basic ' + base64.b64encode(b'jconstant:rayjandom').decode(), + 'Destination': sbox.repo_url + '/!svn/txr/' + txn_name + '/tab%09name' + } + h.request('COPY', sbox.repo_url + '/!svn/rvr/1/iota', None, headers) + r = h.getresponse() + if r.status != httplib.BAD_REQUEST: + raise svntest.Failure('Unexpected status: %d %s' % (r.status, r.reason)) + r.read() + ######################################################################## # Run the tests @@ -700,6 +761,7 @@ test_list = [ None, propfind_allprop, propfind_propname, last_modified_header, + create_name_with_control_chars, ] serial_only = True -- 2.33.0