From 149e299cd7eaadc8248480300b6e13b097c5b3fa Mon Sep 17 00:00:00 2001 From: Jiaying Song Date: Fri, 13 Dec 2024 12:19:43 +0800 Subject: [PATCH] Fix CVE-2024-46901 It has been discovered that the patch for CVE-2013-1968 was incomplete and unintentionally left mod_dav_svn vulnerable to control characters in filenames. Upstream-Status: Backport [https://subversion.apache.org/security/CVE-2024-46901-advisory.txt] CVE: CVE-2024-46901 Signed-off-by: Jiaying Song --- .../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 | 29 +++++++++++++++++++ 5 files changed, 55 insertions(+), 2 deletions(-) diff --git a/subversion/include/private/svn_repos_private.h b/subversion/include/private/svn_repos_private.h index 1fd34e8..1d5fc9c 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 515600d..aad37ee 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 2189de8..119f04b 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 7e9c94b..d2a6aa9 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 8cbd5e7..778ae9b 100644 --- a/subversion/mod_dav_svn/repos.c +++ b/subversion/mod_dav_svn/repos.c @@ -2928,6 +2928,15 @@ 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 +4129,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) @@ -4193,6 +4210,12 @@ copy_resource(const 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); 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 +4453,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 */ -- 2.25.1