|
|
6ec2cd |
|
|
|
6ec2cd |
https://bugzilla.redhat.com/show_bug.cgi?id=17330884
|
|
|
6ec2cd |
|
|
|
6ec2cd |
https://subversion.apache.org/security/CVE-2018-11782-advisory.txt
|
|
|
6ec2cd |
|
|
|
6ec2cd |
Fixes for CVE-2018-11782, svnserve get-deleted-rev assertion failure.
|
|
|
6ec2cd |
|
|
|
6ec2cd |
The svn protocol prototype for get-deleted-rev does not allow for a reply of
|
|
|
6ec2cd |
SVN_INVALID_REVNUM directly. A query having such an answer previously caused
|
|
|
6ec2cd |
the server to raise an assertion failure which could crash the whole process
|
|
|
6ec2cd |
or a thread or child process of it, depending on the build configuration of
|
|
|
6ec2cd |
the server.
|
|
|
6ec2cd |
|
|
|
6ec2cd |
To work around the problem without changing the protocol, we re-purpose the
|
|
|
6ec2cd |
obsolete error code 'SVN_ERR_ENTRY_MISSING_REVISION' to communicate this
|
|
|
6ec2cd |
'not deleted' reply to the client.
|
|
|
6ec2cd |
|
|
|
6ec2cd |
- With a new client against a new server, such queries are now handled
|
|
|
6ec2cd |
correctly.
|
|
|
6ec2cd |
|
|
|
6ec2cd |
- With an old client against a new server, the client will report a more
|
|
|
6ec2cd |
informative error message, and the server will not crash.
|
|
|
6ec2cd |
|
|
|
6ec2cd |
- With a new client against an old server, the behaviour is the same as
|
|
|
6ec2cd |
with an old client against an old server.
|
|
|
6ec2cd |
|
|
|
6ec2cd |
In addition, this fixes a similar problem whereby any regular error response
|
|
|
6ec2cd |
to a 'get-deleted-rev' query resulted in the server closing the connection,
|
|
|
6ec2cd |
process and/or thread (again depending on the build configuration). Now such
|
|
|
6ec2cd |
errors are correctly passed back to the client.
|
|
|
6ec2cd |
|
|
|
6ec2cd |
* subversion/libsvn_ra_svn/client.c
|
|
|
6ec2cd |
(ra_svn_get_deleted_rev): Detect error SVN_ERR_ENTRY_MISSING_REVISION
|
|
|
6ec2cd |
and convert it to a response of SVN_INVALID_REVNUM.
|
|
|
6ec2cd |
|
|
|
6ec2cd |
* subversion/svnserve/serve.c
|
|
|
6ec2cd |
(get_deleted_rev): Respond with error SVN_ERR_ENTRY_MISSING_REVISION
|
|
|
6ec2cd |
instead of an assertion failure if the answer is SVN_INVALID_REVNUM.
|
|
|
6ec2cd |
If svn_repos_deleted_rev() returns an error, pass that error back to
|
|
|
6ec2cd |
the client.
|
|
|
6ec2cd |
|
|
|
6ec2cd |
* subversion/tests/libsvn_ra/ra-test.c
|
|
|
6ec2cd |
(commit_two_changes): New.
|
|
|
6ec2cd |
(test_get_deleted_rev_no_delete,
|
|
|
6ec2cd |
test_get_deleted_rev_errors): New tests.
|
|
|
6ec2cd |
(test_funcs): Run them.
|
|
|
6ec2cd |
--This line, and those below, will be ignored--
|
|
|
6ec2cd |
|
|
|
6ec2cd |
Index: subversion/libsvn_ra_svn/client.c
|
|
|
6ec2cd |
===================================================================
|
|
|
6ec2cd |
--- subversion-1.10.2/subversion/libsvn_ra_svn/client.c.cve11782
|
|
|
6ec2cd |
+++ subversion-1.10.2/subversion/libsvn_ra_svn/client.c
|
|
|
6ec2cd |
@@ -3105,6 +3105,7 @@
|
|
|
6ec2cd |
{
|
|
|
6ec2cd |
svn_ra_svn__session_baton_t *sess_baton = session->priv;
|
|
|
6ec2cd |
svn_ra_svn_conn_t *conn = sess_baton->conn;
|
|
|
6ec2cd |
+ svn_error_t *err;
|
|
|
6ec2cd |
|
|
|
6ec2cd |
path = reparent_path(session, path, pool);
|
|
|
6ec2cd |
|
|
|
6ec2cd |
@@ -3116,8 +3117,20 @@
|
|
|
6ec2cd |
SVN_ERR(handle_unsupported_cmd(handle_auth_request(sess_baton, pool),
|
|
|
6ec2cd |
N_("'get-deleted-rev' not implemented")));
|
|
|
6ec2cd |
|
|
|
6ec2cd |
- return svn_error_trace(svn_ra_svn__read_cmd_response(conn, pool, "r",
|
|
|
6ec2cd |
- revision_deleted));
|
|
|
6ec2cd |
+ err = svn_error_trace(svn_ra_svn__read_cmd_response(conn, pool, "r",
|
|
|
6ec2cd |
+ revision_deleted));
|
|
|
6ec2cd |
+ /* The protocol does not allow for a reply of SVN_INVALID_REVNUM directly.
|
|
|
6ec2cd |
+ Instead, a new enough server returns SVN_ERR_ENTRY_MISSING_REVISION to
|
|
|
6ec2cd |
+ indicate the answer to the query is SVN_INVALID_REVNUM. (An older server
|
|
|
6ec2cd |
+ closes the connection and returns SVN_ERR_RA_SVN_CONNECTION_CLOSED.) */
|
|
|
6ec2cd |
+ if (err && err->apr_err == SVN_ERR_ENTRY_MISSING_REVISION)
|
|
|
6ec2cd |
+ {
|
|
|
6ec2cd |
+ *revision_deleted = SVN_INVALID_REVNUM;
|
|
|
6ec2cd |
+ svn_error_clear(err);
|
|
|
6ec2cd |
+ }
|
|
|
6ec2cd |
+ else
|
|
|
6ec2cd |
+ SVN_ERR(err);
|
|
|
6ec2cd |
+ return SVN_NO_ERROR;
|
|
|
6ec2cd |
}
|
|
|
6ec2cd |
|
|
|
6ec2cd |
static svn_error_t *
|
|
|
6ec2cd |
--- subversion-1.10.2/subversion/svnserve/serve.c.cve11782
|
|
|
6ec2cd |
+++ subversion-1.10.2/subversion/svnserve/serve.c
|
|
|
6ec2cd |
@@ -3505,8 +3505,21 @@
|
|
|
6ec2cd |
svn_relpath_canonicalize(path, pool), pool);
|
|
|
6ec2cd |
SVN_ERR(log_command(b, conn, pool, "get-deleted-rev"));
|
|
|
6ec2cd |
SVN_ERR(trivial_auth_request(conn, pool, b));
|
|
|
6ec2cd |
- SVN_ERR(svn_repos_deleted_rev(b->repository->fs, full_path, peg_revision,
|
|
|
6ec2cd |
- end_revision, &revision_deleted, pool));
|
|
|
6ec2cd |
+ SVN_CMD_ERR(svn_repos_deleted_rev(b->repository->fs, full_path, peg_revision,
|
|
|
6ec2cd |
+ end_revision, &revision_deleted, pool));
|
|
|
6ec2cd |
+
|
|
|
6ec2cd |
+ /* The protocol does not allow for a reply of SVN_INVALID_REVNUM directly.
|
|
|
6ec2cd |
+ Instead, return SVN_ERR_ENTRY_MISSING_REVISION. A new enough client
|
|
|
6ec2cd |
+ knows that this means the answer to the query is SVN_INVALID_REVNUM.
|
|
|
6ec2cd |
+ (An older client reports this as an error.) */
|
|
|
6ec2cd |
+ if (revision_deleted == SVN_INVALID_REVNUM)
|
|
|
6ec2cd |
+ SVN_CMD_ERR(svn_error_createf(SVN_ERR_ENTRY_MISSING_REVISION, NULL,
|
|
|
6ec2cd |
+ "svn protocol command 'get-deleted-rev': "
|
|
|
6ec2cd |
+ "path '%s' was not deleted in r%ld-%ld; "
|
|
|
6ec2cd |
+ "NOTE: newer clients handle this case "
|
|
|
6ec2cd |
+ "and do not report it as an error",
|
|
|
6ec2cd |
+ full_path, peg_revision, end_revision));
|
|
|
6ec2cd |
+
|
|
|
6ec2cd |
SVN_ERR(svn_ra_svn__write_cmd_response(conn, pool, "r", revision_deleted));
|
|
|
6ec2cd |
return SVN_NO_ERROR;
|
|
|
6ec2cd |
}
|
|
|
6ec2cd |
--- subversion-1.10.2/subversion/tests/libsvn_ra/ra-test.c.cve11782
|
|
|
6ec2cd |
+++ subversion-1.10.2/subversion/tests/libsvn_ra/ra-test.c
|
|
|
6ec2cd |
@@ -94,6 +94,41 @@
|
|
|
6ec2cd |
return SVN_NO_ERROR;
|
|
|
6ec2cd |
}
|
|
|
6ec2cd |
|
|
|
6ec2cd |
+/* Commit two revisions: add 'B', then delete 'A' */
|
|
|
6ec2cd |
+static svn_error_t *
|
|
|
6ec2cd |
+commit_two_changes(svn_ra_session_t *session,
|
|
|
6ec2cd |
+ apr_pool_t *pool)
|
|
|
6ec2cd |
+{
|
|
|
6ec2cd |
+ apr_hash_t *revprop_table = apr_hash_make(pool);
|
|
|
6ec2cd |
+ const svn_delta_editor_t *editor;
|
|
|
6ec2cd |
+ void *edit_baton;
|
|
|
6ec2cd |
+ void *root_baton, *dir_baton;
|
|
|
6ec2cd |
+
|
|
|
6ec2cd |
+ /* mkdir B */
|
|
|
6ec2cd |
+ SVN_ERR(svn_ra_get_commit_editor3(session, &editor, &edit_baton,
|
|
|
6ec2cd |
+ revprop_table,
|
|
|
6ec2cd |
+ NULL, NULL, NULL, TRUE, pool));
|
|
|
6ec2cd |
+ SVN_ERR(editor->open_root(edit_baton, SVN_INVALID_REVNUM,
|
|
|
6ec2cd |
+ pool, &root_baton));
|
|
|
6ec2cd |
+ SVN_ERR(editor->add_directory("B", root_baton, NULL, SVN_INVALID_REVNUM,
|
|
|
6ec2cd |
+ pool, &dir_baton));
|
|
|
6ec2cd |
+ SVN_ERR(editor->close_directory(dir_baton, pool));
|
|
|
6ec2cd |
+ SVN_ERR(editor->close_directory(root_baton, pool));
|
|
|
6ec2cd |
+ SVN_ERR(editor->close_edit(edit_baton, pool));
|
|
|
6ec2cd |
+
|
|
|
6ec2cd |
+ /* delete A */
|
|
|
6ec2cd |
+ SVN_ERR(svn_ra_get_commit_editor3(session, &editor, &edit_baton,
|
|
|
6ec2cd |
+ revprop_table,
|
|
|
6ec2cd |
+ NULL, NULL, NULL, TRUE, pool));
|
|
|
6ec2cd |
+ SVN_ERR(editor->open_root(edit_baton, SVN_INVALID_REVNUM,
|
|
|
6ec2cd |
+ pool, &root_baton));
|
|
|
6ec2cd |
+ SVN_ERR(editor->delete_entry("A", SVN_INVALID_REVNUM, root_baton, pool));
|
|
|
6ec2cd |
+ SVN_ERR(editor->close_directory(root_baton, pool));
|
|
|
6ec2cd |
+ SVN_ERR(editor->close_edit(edit_baton, pool));
|
|
|
6ec2cd |
+
|
|
|
6ec2cd |
+ return SVN_NO_ERROR;
|
|
|
6ec2cd |
+}
|
|
|
6ec2cd |
+
|
|
|
6ec2cd |
static svn_error_t *
|
|
|
6ec2cd |
commit_tree(svn_ra_session_t *session,
|
|
|
6ec2cd |
apr_pool_t *pool)
|
|
|
6ec2cd |
@@ -1784,6 +1819,56 @@
|
|
|
6ec2cd |
return SVN_NO_ERROR;
|
|
|
6ec2cd |
}
|
|
|
6ec2cd |
|
|
|
6ec2cd |
+/* Cases of 'get-deleted-rev' that should return SVN_INVALID_REVNUM. */
|
|
|
6ec2cd |
+static svn_error_t *
|
|
|
6ec2cd |
+test_get_deleted_rev_no_delete(const svn_test_opts_t *opts,
|
|
|
6ec2cd |
+ apr_pool_t *pool)
|
|
|
6ec2cd |
+{
|
|
|
6ec2cd |
+ svn_ra_session_t *ra_session;
|
|
|
6ec2cd |
+ svn_revnum_t revision_deleted;
|
|
|
6ec2cd |
+
|
|
|
6ec2cd |
+ SVN_ERR(make_and_open_repos(&ra_session,
|
|
|
6ec2cd |
+ "test-repo-get-deleted-rev-no-delete", opts,
|
|
|
6ec2cd |
+ pool));
|
|
|
6ec2cd |
+ SVN_ERR(commit_changes(ra_session, pool));
|
|
|
6ec2cd |
+ SVN_ERR(commit_two_changes(ra_session, pool));
|
|
|
6ec2cd |
+
|
|
|
6ec2cd |
+ /* expect 'no deletion' in the range up to r2, when it is deleted in r3 */
|
|
|
6ec2cd |
+ /* This was failing over RA-SVN where the 'get-deleted-rev' wire command's
|
|
|
6ec2cd |
+ prototype cannot directly represent that result. A new enough client and
|
|
|
6ec2cd |
+ server collaborate on a work-around implemented using an error code. */
|
|
|
6ec2cd |
+ SVN_ERR(svn_ra_get_deleted_rev(ra_session, "A", 1, 2,
|
|
|
6ec2cd |
+ &revision_deleted, pool));
|
|
|
6ec2cd |
+ SVN_TEST_INT_ASSERT(revision_deleted, SVN_INVALID_REVNUM);
|
|
|
6ec2cd |
+
|
|
|
6ec2cd |
+ /* this connection should still be open: a simple case should still work */
|
|
|
6ec2cd |
+ SVN_ERR(svn_ra_get_deleted_rev(ra_session, "A", 1, 3,
|
|
|
6ec2cd |
+ &revision_deleted, pool));
|
|
|
6ec2cd |
+ SVN_TEST_INT_ASSERT(revision_deleted, 3);
|
|
|
6ec2cd |
+
|
|
|
6ec2cd |
+ return SVN_NO_ERROR;
|
|
|
6ec2cd |
+}
|
|
|
6ec2cd |
+
|
|
|
6ec2cd |
+/* Cases of 'get-deleted-rev' that should return an error. */
|
|
|
6ec2cd |
+static svn_error_t *
|
|
|
6ec2cd |
+test_get_deleted_rev_errors(const svn_test_opts_t *opts,
|
|
|
6ec2cd |
+ apr_pool_t *pool)
|
|
|
6ec2cd |
+{
|
|
|
6ec2cd |
+ svn_ra_session_t *ra_session;
|
|
|
6ec2cd |
+ svn_revnum_t revision_deleted;
|
|
|
6ec2cd |
+
|
|
|
6ec2cd |
+ SVN_ERR(make_and_open_repos(&ra_session,
|
|
|
6ec2cd |
+ "test-repo-get-deleted-rev-errors", opts, pool));
|
|
|
6ec2cd |
+ SVN_ERR(commit_changes(ra_session, pool));
|
|
|
6ec2cd |
+
|
|
|
6ec2cd |
+ /* expect an error when searching up to r3, when repository head is r1 */
|
|
|
6ec2cd |
+ SVN_TEST_ASSERT_ERROR(svn_ra_get_deleted_rev(ra_session, "A", 1, 3,
|
|
|
6ec2cd |
+ &revision_deleted, pool),
|
|
|
6ec2cd |
+ SVN_ERR_FS_NO_SUCH_REVISION);
|
|
|
6ec2cd |
+
|
|
|
6ec2cd |
+ return SVN_NO_ERROR;
|
|
|
6ec2cd |
+}
|
|
|
6ec2cd |
+
|
|
|
6ec2cd |
|
|
|
6ec2cd |
/* The test table. */
|
|
|
6ec2cd |
|
|
|
6ec2cd |
@@ -1820,6 +1905,10 @@
|
|
|
6ec2cd |
"check how last change applies to empty commit"),
|
|
|
6ec2cd |
SVN_TEST_OPTS_PASS(commit_locked_file,
|
|
|
6ec2cd |
"check commit editor for a locked file"),
|
|
|
6ec2cd |
+ SVN_TEST_OPTS_PASS(test_get_deleted_rev_no_delete,
|
|
|
6ec2cd |
+ "test get-deleted-rev no delete"),
|
|
|
6ec2cd |
+ SVN_TEST_OPTS_PASS(test_get_deleted_rev_errors,
|
|
|
6ec2cd |
+ "test get-deleted-rev errors"),
|
|
|
6ec2cd |
SVN_TEST_NULL
|
|
|
6ec2cd |
};
|
|
|
6ec2cd |
|