Subversion HTTP servers allow spoofing svn:author property values for new revisions. Summary: ======== Subversion's mod_dav_svn server allows setting arbitrary svn:author property values when committing new revisions. This can be accomplished using a specially crafted sequence of requests. An evil-doer can fake svn:author values on his commits. However, as authorization rules are applied to the evil-doer's true username, forged svn:author values can only happen on commits that touch the paths the evil-doer has write access to. Doing so does not grant any additional access and does not circumvent the standard Apache authentication or authorization mechanisms. Still, an ability to spoof svn:author property values can impact data integrity in environments that rely on these values. There are no known instances of the problem being exploited in the wild, but an exploit has been tested. Known vulnerable: ================= Subversion HTTPD servers 1.5.0 through 1.7.19 (inclusive) Subversion HTTPD servers 1.8.0 through 1.8.11 (inclusive) Known fixed: ============ Subversion 1.7.20 Subversion 1.8.13 svnserve (any version) is not vulnerable Subversion 1.8.12 was not publicly released. Details: ======== The Subversion http://-based protocol used for communicating with a Subversion mod_dav_svn server has two versions, v1 and v2. The v2 protocol was added in Subversion 1.7.0, but the server allows using both protocol versions for compatibility reasons. When a commit happens, the client sends a sequence of requests (POST, PUT, MERGE, etc.) that depend on the negotiated protocol version. Usually, a server uses the name of the authenticated user as the svn:author value for a new revision. However, with a specially handcrafted v1 request sequence, a client can instruct the server to use the svn:author property that she/he provided. In this case, the server will use an arbitrary value coming from the client instead of the svn:author value originating from the authentication mechanism. Severity: ========= CVSSv2 Base Score: 3.5 CVSSv2 Base Vector: AV:N/AC:M/Au:S/C:N/I:P/A:N We consider this to be a medium risk vulnerability. An attacker needs to have commit access to the repository to exploit the vulnerability. The ability to spoof svn:author property values can impact data integrity in environments that expect the values to denote the actual commit author. The real ID of the author could still be determined using server access logs. However, it is also possible that a spoofed change could go in unnoticed. Subversion's repository hooks might see the real ID of the author or the forged value, depending on the hook type and the hook contents: - A start-commit hook will see the real username in the USER argument - A start-commit hook will see the real username when performing 'svnlook propget --revprop -t TXN_NAME' - A pre-commit hook will see the forged username when performing 'svnlook propget --revprop -t TXN_NAME' - A post-commit hook will see the forged username when performing 'svnlook propget --revprop -r REV' Unfortunately, no special configuration is required and all mod_dav_svn servers are vulnerable. Recommendations: ================ We recommend all users to upgrade to Subversion 1.8.13. Users of Subversion 1.7.x or 1.8.x who are unable to upgrade may apply the included patch. New Subversion packages can be found at: http://subversion.apache.org/packages.html No workaround is available. References: =========== CVE-2015-0251 (Subversion) Reported by: ============ Bruno Luiz, d4t Patches: ======== Patch against 1.7.19: [[[ Index: subversion/mod_dav_svn/deadprops.c =================================================================== --- subversion/mod_dav_svn/deadprops.c (revision 1660122) +++ subversion/mod_dav_svn/deadprops.c (working copy) @@ -160,6 +160,23 @@ get_value(dav_db *db, const dav_prop_name *name, s } +static svn_error_t * +change_txn_prop(svn_fs_txn_t *txn, + const char *propname, + const svn_string_t *value, + apr_pool_t *scratch_pool) +{ + if (strcmp(propname, SVN_PROP_REVISION_AUTHOR) == 0) + return svn_error_create(SVN_ERR_RA_DAV_REQUEST_FAILED, NULL, + "Attempted to modify 'svn:author' property " + "on a transaction"); + + SVN_ERR(svn_repos_fs_change_txn_prop(txn, propname, value, scratch_pool)); + + return SVN_NO_ERROR; +} + + static dav_error * save_value(dav_db *db, const dav_prop_name *name, const svn_string_t *const *old_value_p, @@ -210,9 +227,8 @@ save_value(dav_db *db, const dav_prop_name *name, { if (db->resource->working) { - serr = svn_repos_fs_change_txn_prop(resource->info->root.txn, - propname, value, - subpool); + serr = change_txn_prop(resource->info->root.txn, propname, + value, subpool); } else { @@ -251,8 +267,8 @@ save_value(dav_db *db, const dav_prop_name *name, } else if (resource->info->restype == DAV_SVN_RESTYPE_TXN_COLLECTION) { - serr = svn_repos_fs_change_txn_prop(resource->info->root.txn, - propname, value, subpool); + serr = change_txn_prop(resource->info->root.txn, propname, + value, subpool); } else { @@ -561,8 +577,8 @@ db_remove(dav_db *db, const dav_prop_name *name) /* Working Baseline or Working (Version) Resource */ if (db->resource->baselined) if (db->resource->working) - serr = svn_repos_fs_change_txn_prop(db->resource->info->root.txn, - propname, NULL, subpool); + serr = change_txn_prop(db->resource->info->root.txn, propname, + NULL, subpool); else /* ### VIOLATING deltaV: you can't proppatch a baseline, it's not a working resource! But this is how we currently ]]] Patch against 1.8.11: [[[ Index: subversion/mod_dav_svn/deadprops.c =================================================================== --- subversion/mod_dav_svn/deadprops.c (revision 1660122) +++ subversion/mod_dav_svn/deadprops.c (working copy) @@ -163,6 +163,23 @@ get_value(dav_db *db, const dav_prop_name *name, s } +static svn_error_t * +change_txn_prop(svn_fs_txn_t *txn, + const char *propname, + const svn_string_t *value, + apr_pool_t *scratch_pool) +{ + if (strcmp(propname, SVN_PROP_REVISION_AUTHOR) == 0) + return svn_error_create(SVN_ERR_RA_DAV_REQUEST_FAILED, NULL, + "Attempted to modify 'svn:author' property " + "on a transaction"); + + SVN_ERR(svn_repos_fs_change_txn_prop(txn, propname, value, scratch_pool)); + + return SVN_NO_ERROR; +} + + static dav_error * save_value(dav_db *db, const dav_prop_name *name, const svn_string_t *const *old_value_p, @@ -213,9 +230,8 @@ save_value(dav_db *db, const dav_prop_name *name, { if (resource->working) { - serr = svn_repos_fs_change_txn_prop(resource->info->root.txn, - propname, value, - subpool); + serr = change_txn_prop(resource->info->root.txn, propname, + value, subpool); } else { @@ -254,8 +270,8 @@ save_value(dav_db *db, const dav_prop_name *name, } else if (resource->info->restype == DAV_SVN_RESTYPE_TXN_COLLECTION) { - serr = svn_repos_fs_change_txn_prop(resource->info->root.txn, - propname, value, subpool); + serr = change_txn_prop(resource->info->root.txn, propname, + value, subpool); } else { @@ -560,8 +576,8 @@ db_remove(dav_db *db, const dav_prop_name *name) /* Working Baseline or Working (Version) Resource */ if (db->resource->baselined) if (db->resource->working) - serr = svn_repos_fs_change_txn_prop(db->resource->info->root.txn, - propname, NULL, subpool); + serr = change_txn_prop(db->resource->info->root.txn, propname, + NULL, subpool); else /* ### VIOLATING deltaV: you can't proppatch a baseline, it's not a working resource! But this is how we currently ]]]