CVE-2021-28544

SVN authz protected copyfrom paths regression Summary: ======== Subversion servers reveal ‘copyfrom’ paths that should be hidden according to configured path-based authorization (authz) rules. When a node has been copied from a protected location, users with access to the copy can see the ‘copyfrom’ path of the original. This also reveals the fact that the node was copied. Only the ‘copyfrom’ path is revealed; not its contents. Both httpd and svnserve servers are vulnerable. Known vulnerable: ================= Subversion httpd servers 1.10.0 through 1.14.1 (inclusive). Subversion svnserve servers 1.10.0 through 1.14.1 (inclusive). Repositories that do not use path-based authorization are not affected. Known fixed: ============ Subversion httpd and svnserve servers 1.14.2. Subversion httpd and svnserve servers 1.10.8. Details: ======== When retrieving log messages, the helper function detect_changed() finds and reports on applicable changes, such as when a node has been added by copying. When authz is used, detect_changed() should omit information on nodes that are unreadable per authz rules. In particular, if a node in a readable location has been copied from an unreadable location, the readable node should be reported but its copyfrom path (the path to the unreadable location) should be omitted. Due to an implementation error, the above-mentioned copyfrom paths are reported even if they should be omitted. Note that only the path itself is reported, not the contents of the file or directory at that path. Attempts to access the contents are met with an authorization error as expected. Example: Suppose a file is copied: svn cp $REPO/private/file.txt $REPO/public -m “Copy file.txt” and the following authz config applies: [repo:/] *=rw [repo:/private] *= With a vulnerable server, ‘svn log’ reveals the existence of the original and its path: svn log $REPO/public --verbose --limit 1 … Changed paths: A /public/file.txt (from /private/file.txt:1) … Non-vulnerable servers do not report this information: … Changed paths: A /public/file.txt … Note that only the path to /private/file.txt is revealed; not its contents. Users who cannot access /private/file.txt according to authz rules will not be able to access its contents. Severity: ========= CVSSv3.1 Base Score: 4.3 CVSSv3.1 Base Vector: AV:N/AC:L/PR:L/UI:N/S:U/C:L/I:N/A:N Recommendations: ================ We recommend all users to upgrade to a known fixed release of the Subversion server. Users who are unable to upgrade may apply the included patches. References: =========== CVE-2021-28544 (Subversion) Reported by: ============ Evgeny Kotkov, visualsvn.com Patches: ======== Patch against Subversion 1.14.1: [[[ Index: subversion/libsvn_repos/log.c =================================================================== — subversion/libsvn_repos/log.c (revision 1890531) +++ subversion/libsvn_repos/log.c (working copy) @@ -337,43 +337,37 @@ detect_changed(svn_repos_revision_access_level_t * if ( (change->change_kind == svn_fs_path_change_add) || (change->change_kind == svn_fs_path_change_replace)) { - const char *copyfrom_path = change->copyfrom_path; - svn_revnum_t copyfrom_rev = change->copyfrom_rev; - /* the following is a potentially expensive operation since on FSFS we will follow the DAG from ROOT to PATH and that requires actually reading the directories along the way. */ if (!change->copyfrom_known) { - SVN_ERR(svn_fs_copied_from(©from_rev, ©from_path, + SVN_ERR(svn_fs_copied_from(&change->copyfrom_rev, &change->copyfrom_path, root, path, iterpool)); change->copyfrom_known = TRUE; } - if (copyfrom_path && SVN_IS_VALID_REVNUM(copyfrom_rev)) + if (change->copyfrom_path && SVN_IS_VALID_REVNUM(change->copyfrom_rev)) { - svn_boolean_t readable = TRUE; - if (callbacks->authz_read_func) { svn_fs_root_t *copyfrom_root; + svn_boolean_t readable; SVN_ERR(svn_fs_revision_root(©from_root, fs, - copyfrom_rev, iterpool)); + change->copyfrom_rev, iterpool)); SVN_ERR(callbacks->authz_read_func(&readable, copyfrom_root, - copyfrom_path, + change->copyfrom_path, callbacks->authz_read_baton, iterpool)); if (! readable) - found_unreadable = TRUE; + { + found_unreadable = TRUE; + change->copyfrom_path = NULL; + change->copyfrom_rev = SVN_INVALID_REVNUM; + } } - - if (readable) - { - change->copyfrom_path = copyfrom_path; - change->copyfrom_rev = copyfrom_rev; - } } } Index: subversion/tests/cmdline/authz_tests.py =================================================================== — subversion/tests/cmdline/authz_tests.py (revision 1890531) +++ subversion/tests/cmdline/authz_tests.py (working copy) @@ -1731,7 +1731,62 @@ def empty_group(sbox): '–username’, svntest.main.wc_author, sbox.repo_url) +# test for the bug also known as CVE-2021-28544 +@Skip(svntest.main.is_ra_type_file) +def log_inaccessible_copyfrom(sbox): + “log doesn’t leak inaccessible copyfrom paths” + sbox.build(empty=True) + sbox.simple_add_text(‘secret’, ‘private’) + sbox.simple_commit(message=’log message for r1’) + sbox.simple_copy(‘private’, ‘public’) + sbox.simple_commit(message=’log message for r2’) + + svntest.actions.enable_revprop_changes(sbox.repo_dir) + # Remove svn:date and svn:author for predictable output. + svntest.actions.run_and_verify_svn(None, [], 'propdel’, '–revprop’, + '-r2’, 'svn:date’, sbox.repo_url) + svntest.actions.run_and_verify_svn(None, [], 'propdel’, '–revprop’, + '-r2’, 'svn:author’, sbox.repo_url) + + write_restrictive_svnserve_conf(sbox.repo_dir) + + # First test with blanket access. + write_authz_file(sbox, + {"/" : "* = rw"}) + expected_output = svntest.verify.ExpectedOutput([ + "------------------------------------------------------------------------\n", + "r2 | (no author) | (no date) | 1 line\n", + "Changed paths:\n", + " A /public (from /private:1)\n", + "\n", + “log message for r2\n", + "------------------------------------------------------------------------\n", + ]) + svntest.actions.run_and_verify_svn(expected_output, [], + 'log’, '-r2’, '-v’, + sbox.repo_url) + + # Now test with an inaccessible copy source (/private). + write_authz_file(sbox, + {"/” : “* = rw"}, + {"/private” : "* ="}) + expected_output = svntest.verify.ExpectedOutput([ + "------------------------------------------------------------------------\n", + "r2 | (no author) | (no date) | 1 line\n", + "Changed paths:\n", + # The copy is shown as a plain add with no copyfrom info. + " A /public\n", + "\n", + # No log message, as the revision is only partially visible. + “\n", + "------------------------------------------------------------------------\n", + ]) + svntest.actions.run_and_verify_svn(expected_output, [], + 'log’, '-r2’, '-v’, + sbox.repo_url) + + ######################################################################## # Run the tests @@ -1771,6 +1826,7 @@ test_list = [ None, inverted_group_membership, group_member_empty_string, empty_group, + log_inaccessible_copyfrom, ] serial_only = True ]]] Patch against Subversion 1.10.7: [[[ Index: subversion/libsvn_repos/log.c =================================================================== — subversion/libsvn_repos/log.c (revision 1890531) +++ subversion/libsvn_repos/log.c (working copy) @@ -337,43 +337,37 @@ detect_changed(svn_repos_revision_access_level_t * if ( (change->change_kind == svn_fs_path_change_add) || (change->change_kind == svn_fs_path_change_replace)) { - const char *copyfrom_path = change->copyfrom_path; - svn_revnum_t copyfrom_rev = change->copyfrom_rev; - /* the following is a potentially expensive operation since on FSFS we will follow the DAG from ROOT to PATH and that requires actually reading the directories along the way. */ if (!change->copyfrom_known) { - SVN_ERR(svn_fs_copied_from(©from_rev, ©from_path, + SVN_ERR(svn_fs_copied_from(&change->copyfrom_rev, &change->copyfrom_path, root, path, iterpool)); change->copyfrom_known = TRUE; } - if (copyfrom_path && SVN_IS_VALID_REVNUM(copyfrom_rev)) + if (change->copyfrom_path && SVN_IS_VALID_REVNUM(change->copyfrom_rev)) { - svn_boolean_t readable = TRUE; - if (callbacks->authz_read_func) { svn_fs_root_t *copyfrom_root; + svn_boolean_t readable; SVN_ERR(svn_fs_revision_root(©from_root, fs, - copyfrom_rev, iterpool)); + change->copyfrom_rev, iterpool)); SVN_ERR(callbacks->authz_read_func(&readable, copyfrom_root, - copyfrom_path, + change->copyfrom_path, callbacks->authz_read_baton, iterpool)); if (! readable) - found_unreadable = TRUE; + { + found_unreadable = TRUE; + change->copyfrom_path = NULL; + change->copyfrom_rev = SVN_INVALID_REVNUM; + } } - - if (readable) - { - change->copyfrom_path = copyfrom_path; - change->copyfrom_rev = copyfrom_rev; - } } } Index: subversion/tests/cmdline/authz_tests.py =================================================================== — subversion/tests/cmdline/authz_tests.py (revision 1890531) +++ subversion/tests/cmdline/authz_tests.py (working copy) @@ -1710,7 +1710,62 @@ def group_member_empty_string(sbox): '–username’, svntest.main.wc_author, sbox.repo_url) +# test for the bug also known as CVE-2021-28544 +@Skip(svntest.main.is_ra_type_file) +def log_inaccessible_copyfrom(sbox): + “log doesn’t leak inaccessible copyfrom paths” + sbox.build(empty=True) + sbox.simple_add_text(‘secret’, ‘private’) + sbox.simple_commit(message=’log message for r1’) + sbox.simple_copy(‘private’, ‘public’) + sbox.simple_commit(message=’log message for r2’) + + svntest.actions.enable_revprop_changes(sbox.repo_dir) + # Remove svn:date and svn:author for predictable output. + svntest.actions.run_and_verify_svn(None, [], 'propdel’, '–revprop’, + '-r2’, 'svn:date’, sbox.repo_url) + svntest.actions.run_and_verify_svn(None, [], 'propdel’, '–revprop’, + '-r2’, 'svn:author’, sbox.repo_url) + + write_restrictive_svnserve_conf(sbox.repo_dir) + + # First test with blanket access. + write_authz_file(sbox, + {"/” : "* = rw"}) + expected_output = svntest.verify.ExpectedOutput([ + "------------------------------------------------------------------------\n", + "r2 | (no author) | (no date) | 1 line\n", + "Changed paths:\n", + " A /public (from /private:1)\n", + "\n", + “log message for r2\n", + "------------------------------------------------------------------------\n", + ]) + svntest.actions.run_and_verify_svn(expected_output, [], + 'log’, '-r2’, '-v’, + sbox.repo_url) + + # Now test with an inaccessible copy source (/private). + write_authz_file(sbox, + {"/” : “* = rw"}, + {"/private” : "* ="}) + expected_output = svntest.verify.ExpectedOutput([ + "------------------------------------------------------------------------\n", + "r2 | (no author) | (no date) | 1 line\n", + "Changed paths:\n", + # The copy is shown as a plain add with no copyfrom info. + " A /public\n", + "\n", + # No log message, as the revision is only partially visible. + "\n", + "------------------------------------------------------------------------\n", + ]) + svntest.actions.run_and_verify_svn(expected_output, [], + 'log’, '-r2’, '-v’, + sbox.repo_url) + + ######################################################################## # Run the tests @@ -1749,6 +1804,7 @@ test_list = [ None, remove_access_after_commit, inverted_group_membership, group_member_empty_string, + log_inaccessible_copyfrom, ] serial_only = True ]]]