Package: release.debian.org
Severity: normal
Tags: bookworm
X-Debbugs-Cc: subvers...@packages.debian.org
Control: affects -1 + src:subversion
User: release.debian....@packages.debian.org
Usertags: pu

[ Reason ]
CVE-2024-46901 was issued for subversion. Although it's marked no DSA,
it would be useful to provide the update to the stable release.

https://security-tracker.debian.org/tracker/CVE-2024-46901

[ Impact ]
Malicious subversion clients can cause DoS of mod_dav_svn servers by
making commits which contain control characters in paths or revision
properties.

[ Tests ]
A new test was added by upstream and included in the backport. I've
added a run of the upstream tests over the dav protocol into the package
so the test is exercised.

[ Risks ]
The changes are relatively straight forward. Existing checks from the
fix for the previous CVE have been incorporated in other code paths to
ensure all relevant code paths are protecting against commits with
control characters.

[ Checklist ]
  [x] *all* changes are documented in the d/changelog
  [x] I reviewed all changes and I approve them
  [x] attach debdiff against the package in (old)stable
  [x] the issue is verified as fixed in unstable

[ Changes ]
* Salsa CI config was added to the package. This was previously
  configured in the repo, rather than as a file in the packaging.
* Additional test-specific Build-Depends (apache2-bin, apache2-utils,
  net-tools, and wget) are added to run upstream's "make davautocheck"
  (the target that runs all relevant tests using mod_dav_svn to access
  the repository).
* debian/rules now runs "make davautocheck" in addition to the existing
  "make check" (which uses file:// access to the repository).
* Upstream's fix and respective test are backported

Full commit log is
https://salsa.debian.org/jamessan/subversion/-/compare/debian/1.14.2-4...debian/bookworm

[ Other info ]
The fix was also included in 1.14.5, which is already in testing /
unstable. While verifying the patch and accompanying test for the
bookworm upload, I also discovered that I needed to run davautocheck to
exercise the mod_dav_svn path for interacting with the svn repository.
That is now enabled in sid via 1.14.5-2 and also in this upload.
diffstat for subversion-1.14.2 subversion-1.14.2

 changelog                                      |    9 +
 control                                        |    4 
 gbp.conf                                       |    2 
 patches/backport-fix-for-cve-2024-46901.patch  |  156 +++++++++++++++++++++++++
 patches/backport-test-for-cve-2024-46901.patch |   90 ++++++++++++++
 patches/series                                 |    2 
 rules                                          |    6 
 salsa-ci.yml                                   |    6 
 8 files changed, 274 insertions(+), 1 deletion(-)

diff -Nru subversion-1.14.2/debian/changelog subversion-1.14.2/debian/changelog
--- subversion-1.14.2/debian/changelog  2022-11-12 15:30:30.000000000 -0500
+++ subversion-1.14.2/debian/changelog  2025-01-03 20:39:54.000000000 -0500
@@ -1,3 +1,12 @@
+subversion (1.14.2-4+deb12u1) bookworm; urgency=medium
+
+  * Add salsa-ci config targeting bookworm
+  * Backport fix for CVE-2024-46901
+  * Backport test for CVE-2024-46901
+  * Run davautocheck as part of the build time tests
+
+ -- James McCoy <james...@debian.org>  Fri, 03 Jan 2025 20:39:54 -0500
+
 subversion (1.14.2-4) unstable; urgency=medium
 
   * Backport patch to fix building with swig 4.1 (Closes: #1023529)
diff -Nru subversion-1.14.2/debian/control subversion-1.14.2/debian/control
--- subversion-1.14.2/debian/control    2022-11-12 15:30:30.000000000 -0500
+++ subversion-1.14.2/debian/control    2025-01-03 20:39:54.000000000 -0500
@@ -3,6 +3,8 @@
 Priority: optional
 Maintainer: James McCoy <james...@debian.org>
 Build-Depends: autoconf,
+               apache2-bin <!nocheck>,
+               apache2-utils <!nocheck>,
                bash-completion,
                chrpath,
                debhelper-compat (= 13),
@@ -26,6 +28,7 @@
                libsqlite3-dev (>= 3.8.7),
                libtool,
                libutf8proc-dev,
+               net-tools <!nocheck>,
                perl,
                py3c-dev,
                python3-all-dev,
@@ -33,6 +36,7 @@
                ruby [!ia64 !kfreebsd-i386 !kfreebsd-amd64] 
<!pkg.subversion.noruby>,
                ruby-dev [!ia64 !kfreebsd-i386 !kfreebsd-amd64] 
<!pkg.subversion.noruby>,
                swig (>= 3.0.10),
+               wget <!nocheck>,
                zlib1g-dev
 Build-Conflicts:
  libsvn-dev (<< 1.14~),
diff -Nru subversion-1.14.2/debian/gbp.conf subversion-1.14.2/debian/gbp.conf
--- subversion-1.14.2/debian/gbp.conf   2022-11-12 15:30:30.000000000 -0500
+++ subversion-1.14.2/debian/gbp.conf   2025-01-03 20:39:54.000000000 -0500
@@ -1,5 +1,5 @@
 [DEFAULT]
-debian-branch = debian/sid
+debian-branch = debian/bookworm
 upstream-tag = upstream/%(version)s
 
 sign-tags = True
diff -Nru 
subversion-1.14.2/debian/patches/backport-fix-for-cve-2024-46901.patch 
subversion-1.14.2/debian/patches/backport-fix-for-cve-2024-46901.patch
--- subversion-1.14.2/debian/patches/backport-fix-for-cve-2024-46901.patch      
1969-12-31 19:00:00.000000000 -0500
+++ subversion-1.14.2/debian/patches/backport-fix-for-cve-2024-46901.patch      
2025-01-03 20:39:54.000000000 -0500
@@ -0,0 +1,156 @@
+From: James McCoy <james...@debian.org>
+Date: Wed, 1 Jan 2025 16:05:33 -0500
+X-Dgit-Generated: 1.14.2-4+deb12u1 b5688fe2dcb483bd788fa97648317af43dd94aa6
+Subject: Backport fix for 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.
+
+If a path or a revision-property which contains control characters is
+committed to a repository then SVN operations served by mod_dav_svn
+can be disrupted.
+
+Origin: backport, from CVE-2024-46901 advisory
+Signed-off-by: James McCoy <james...@debian.org>
+
+---
+
+diff --git a/subversion/include/private/svn_repos_private.h 
b/subversion/include/private/svn_repos_private.h
+index 1fd34e8053c..1d5fc9cd6ea 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 515600d4f94..aad37eea69b 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 df876524be5..7da0c069501 100644
+--- a/subversion/libsvn_repos/repos.c
++++ b/subversion/libsvn_repos/repos.c
+@@ -2100,3 +2100,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 7e9c94b64d0..d2a6aa9021e 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 8cbd5e70e69..c411a173b43 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 -Nru 
subversion-1.14.2/debian/patches/backport-test-for-cve-2024-46901.patch 
subversion-1.14.2/debian/patches/backport-test-for-cve-2024-46901.patch
--- subversion-1.14.2/debian/patches/backport-test-for-cve-2024-46901.patch     
1969-12-31 19:00:00.000000000 -0500
+++ subversion-1.14.2/debian/patches/backport-test-for-cve-2024-46901.patch     
2025-01-03 20:39:54.000000000 -0500
@@ -0,0 +1,90 @@
+From: James McCoy <james...@debian.org>
+Date: Wed, 1 Jan 2025 19:44:05 -0500
+X-Dgit-Generated: 1.14.2-4+deb12u1 c1b1990ed2cea99f5ec3e0797cfafa014bca174b
+Subject: Backport test for CVE-2024-46901
+
+Origin: backport, from CVE-2024-46901 advisory
+Signed-off-by: James McCoy <james...@debian.org>
+
+---
+
+diff --git a/subversion/tests/cmdline/mod_dav_svn_tests.py 
b/subversion/tests/cmdline/mod_dav_svn_tests.py
+index 3aa6cad5cf1..eb8b7aa36c4 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
+ 
diff -Nru subversion-1.14.2/debian/patches/series 
subversion-1.14.2/debian/patches/series
--- subversion-1.14.2/debian/patches/series     2022-11-12 15:30:30.000000000 
-0500
+++ subversion-1.14.2/debian/patches/series     2025-01-03 20:39:54.000000000 
-0500
@@ -11,3 +11,5 @@
 workaround_EINVAL_on_kfreebsd
 use-python3-as-the-interpreter-now-for-tests-not-python.patch
 swig-py-fix-conditionals-by-swig-version.patch
+backport-fix-for-cve-2024-46901.patch
+backport-test-for-cve-2024-46901.patch
diff -Nru subversion-1.14.2/debian/rules subversion-1.14.2/debian/rules
--- subversion-1.14.2/debian/rules      2022-11-12 15:30:30.000000000 -0500
+++ subversion-1.14.2/debian/rules      2025-01-03 20:39:54.000000000 -0500
@@ -240,6 +240,12 @@
          cat $(DEB_BUILDDIR)/tests.log; \
          exit 1; \
        fi
+       @if ! $(MAKE) -C $(DEB_BUILDDIR) davautocheck $(check_defs) 
PARALLEL=$(DEB_OPT_PARALLEL) APACHE_MPM=event; then \
+         echo "###################################################"; \
+         echo "mod_dav testsuite failed, 'tests.log' follows:"; echo; \
+         cat $(DEB_BUILDDIR)/tests.log; \
+         exit 1; \
+       fi
 
 check-swig-rb check-javahl:
        $(MAKE_B) $@ $(check_defs) PARALLEL=$(DEB_OPT_PARALLEL)
diff -Nru subversion-1.14.2/debian/salsa-ci.yml 
subversion-1.14.2/debian/salsa-ci.yml
--- subversion-1.14.2/debian/salsa-ci.yml       1969-12-31 19:00:00.000000000 
-0500
+++ subversion-1.14.2/debian/salsa-ci.yml       2025-01-03 20:39:54.000000000 
-0500
@@ -0,0 +1,6 @@
+---
+include:
+  - 
https://salsa.debian.org/salsa-ci-team/pipeline/raw/master/recipes/debian.yml
+
+variables:
+  RELEASE: 'bookworm'

Reply via email to