Hi!
I've discovered another crash in lock-related code. Usage of deprecated API
svn_fs_access_add_lock_token() function leads to crash when pre-commit hook is
getting executed.
Function svn_fs_access_add_lock_token() simply calls
svn_fs_access_add_lock_token2() and passes some magic value ((const char *) 1)
as PATH parameter. Such condition is not checked by function
lock_token_content() which is used to prepare STDIN contents for pre-commit
hook:
[[[
static svn_error_t *
lock_token_content(apr_file_t **handle, apr_hash_t *lock_tokens,
apr_pool_t *pool)
{
...
for (hi = apr_hash_first(pool, lock_tokens); hi;
hi = apr_hash_next(hi))
{
const char *token = apr_hash_this_key(hi);
const char *path = apr_hash_this_val(hi);
svn_stringbuf_appendstr(lock_str,
svn_stringbuf_createf(pool, "%s|%s\n",
svn_path_uri_autoescape(path, pool),
token));
}
...
}
]]]
I've attached the patch with crashing test and simple fix, but I'm not really
sure about my solution. A probably better approach would be to replace magic
pointer value by an empty string, but I'm not sure about binary compatibility
etc.
Log message:
[[[
Fix potential crash in libsvn_repos when executing pre-commit hook.
* subversion/subversion/libsvn_repos/hooks.c
(lock_token_content): Add special handling for 'magic' value
((const char *) 1).
* subversion/subversion/tests/libsvn_repos/repos-test.c
(pre_commit_hook_lock_token_without_path): New.
(test_funcs): Add new test.
Patch by: sergey.raevskiy{_AT_}visualsvn.com
]]]
Index: subversion/libsvn_repos/hooks.c
===================================================================
--- subversion/libsvn_repos/hooks.c (revision 1657614)
+++ subversion/libsvn_repos/hooks.c (working copy)
@@ -522,10 +522,19 @@ lock_token_content(apr_file_t **handle, apr_hash_t
const char *token = apr_hash_this_key(hi);
const char *path = apr_hash_this_val(hi);
+ if (path == (const char *) 1)
+ {
+ /* Special handling for svn_fs_access_t * created by using deprecated
+ svn_fs_access_add_lock_token() function. */
+ path = "";
+ }
+ else
+ {
+ path = svn_path_uri_autoescape(path, pool);
+ }
+
svn_stringbuf_appendstr(lock_str,
- svn_stringbuf_createf(pool, "%s|%s\n",
- svn_path_uri_autoescape(path, pool),
- token));
+ svn_stringbuf_createf(pool, "%s|%s\n", path, token));
}
svn_stringbuf_appendcstr(lock_str, "\n");
Index: subversion/tests/libsvn_repos/repos-test.c
===================================================================
--- subversion/tests/libsvn_repos/repos-test.c (revision 1657614)
+++ subversion/tests/libsvn_repos/repos-test.c (working copy)
@@ -3562,6 +3562,45 @@ test_repos_fs_type(const svn_test_opts_t *opts,
return SVN_NO_ERROR;
}
+static svn_error_t *
+pre_commit_hook_lock_token_without_path(const svn_test_opts_t *opts,
+ apr_pool_t *pool)
+{
+ svn_repos_t *repos;
+ svn_fs_access_t *access;
+ svn_fs_txn_t *txn;
+ svn_fs_root_t *root;
+ const char *conflict;
+ svn_revnum_t new_rev;
+
+ /* Create test repository. */
+ SVN_ERR(svn_test__create_repos(&repos, "pre_commit_hook_tokens_test",
+ opts, pool));
+
+ /* Set an empty pre-commit hook. */
+ SVN_ERR(svn_io_file_create(svn_repos_pre_commit_hook(repos, pool),
+ "exit 0", pool));
+ SVN_ERR(svn_io_set_file_executable(svn_repos_pre_commit_hook(repos, pool),
+ TRUE, FALSE, pool));
+
+ /* Set some access context using svn_fs_access_add_lock_token(). */
+ SVN_ERR(svn_fs_create_access(&access, "jrandom", pool));
+ SVN_ERR(svn_fs_access_add_lock_token(access, "opaquelocktoken:abc"));
+ SVN_ERR(svn_fs_set_access(svn_repos_fs(repos), access));
+
+ /* Try to commit a new revision. */
+ SVN_ERR(svn_repos_fs_begin_txn_for_commit2(&txn, repos, 0,
+ apr_hash_make(pool), pool));
+ SVN_ERR(svn_fs_txn_root(&root, txn, pool));
+ SVN_ERR(svn_fs_make_dir(root, "/whatever", pool));
+ SVN_ERR(svn_fs_commit_txn(&conflict, &new_rev, txn, pool));
+
+ SVN_TEST_STRING_ASSERT(conflict, NULL);
+ SVN_TEST_ASSERT(new_rev == 1);
+
+ return SVN_NO_ERROR;
+}
+
/* The test table. */
static int max_threads = 4;
@@ -3615,6 +3654,8 @@ static struct svn_test_descriptor_t test_funcs[] =
"test svn_repos__config_pool_*"),
SVN_TEST_OPTS_PASS(test_repos_fs_type,
"test test_repos_fs_type"),
+ SVN_TEST_OPTS_PASS(pre_commit_hook_lock_token_without_path,
+ "test legacy access context api"),
SVN_TEST_NULL
};
