You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@subversion.apache.org by st...@apache.org on 2013/05/22 19:35:00 UTC
svn commit: r1485299 - in /subversion/branches/1.6.x: ./ STATUS
subversion/libsvn_fs_fs/tree.c subversion/tests/libsvn_fs/fs-test.c
Author: stsp
Date: Wed May 22 17:35:00 2013
New Revision: 1485299
URL: http://svn.apache.org/r1485299
Log:
Merge the 1.6.x-issue4340 branch into 1.6.x.
* r1461562, r1461580, r1461701, r1481627
Fix issue #4340, "filenames containing \n corrupt FSFS repositories"
Justification:
Newline characters can severely corrupt FSFS revision files and
should never enter the repository for this reason. See discussion
linked from issue #4340 for more information.
Notes:
r1461701 revises the changes made in the earlier revisions,
and is the result of a long dev@ discussion that eventually concluded
in this subthread: http://svn.haxx.se/dev/archive-2013-04/0056.shtml
This issue can be exploited by people with commit access to corrupt
an FSFS repository, and has been assigned a CVE number: CVE-2013-1968
r1481627 addresses concerns raised by danielsh.
Branch:
^/subversion/branches/1.6.x-issue4340
Votes:
+1: stsp, danielsh, rhuijben
Modified:
subversion/branches/1.6.x/ (props changed)
subversion/branches/1.6.x/STATUS
subversion/branches/1.6.x/subversion/libsvn_fs_fs/tree.c
subversion/branches/1.6.x/subversion/tests/libsvn_fs/fs-test.c
Propchange: subversion/branches/1.6.x/
------------------------------------------------------------------------------
Merged /subversion/trunk:r1461562,1461580,1461701,1481627
Merged /subversion/branches/1.6.x-issue4340:r1461605-1485297
Modified: subversion/branches/1.6.x/STATUS
URL: http://svn.apache.org/viewvc/subversion/branches/1.6.x/STATUS?rev=1485299&r1=1485298&r2=1485299&view=diff
==============================================================================
--- subversion/branches/1.6.x/STATUS (original)
+++ subversion/branches/1.6.x/STATUS Wed May 22 17:35:00 2013
@@ -89,21 +89,3 @@ Veto-blocked changes:
Approved changes:
=================
-
- * r1461562, r1461580, r1461701, r1481627
- Fix issue #4340, "filenames containing \n corrupt FSFS repositories"
- Justification:
- Newline characters can severely corrupt FSFS revision files and
- should never enter the repository for this reason. See discussion
- linked from issue #4340 for more information.
- Notes:
- r1461701 revises the changes made in the earlier revisions,
- and is the result of a long dev@ discussion that eventually concluded
- in this subthread: http://svn.haxx.se/dev/archive-2013-04/0056.shtml
- This issue can be exploited by people with commit access to corrupt
- an FSFS repository, and has been assigned a CVE number: CVE-2013-1968
- r1481627 addresses concerns raised by danielsh.
- Branch:
- ^/subversion/branches/1.6.x-issue4340
- Votes:
- +1: stsp, danielsh, rhuijben
Modified: subversion/branches/1.6.x/subversion/libsvn_fs_fs/tree.c
URL: http://svn.apache.org/viewvc/subversion/branches/1.6.x/subversion/libsvn_fs_fs/tree.c?rev=1485299&r1=1485298&r2=1485299&view=diff
==============================================================================
--- subversion/branches/1.6.x/subversion/libsvn_fs_fs/tree.c (original)
+++ subversion/branches/1.6.x/subversion/libsvn_fs_fs/tree.c Wed May 22 17:35:00 2013
@@ -43,6 +43,7 @@
#include "svn_mergeinfo.h"
#include "svn_fs.h"
#include "svn_props.h"
+#include "svn_ctype.h"
#include "fs.h"
#include "err.h"
@@ -1810,6 +1811,78 @@ fs_dir_entries(apr_hash_t **table_p,
return svn_fs_fs__dag_dir_entries(table_p, node, pool, pool);
}
+/* Return a copy of PATH, allocated from POOL, for which control
+ characters have been escaped using the form \NNN (where NNN is the
+ octal representation of the byte's ordinal value). */
+static const char *
+illegal_path_escape(const char *path, apr_pool_t *pool)
+{
+ svn_stringbuf_t *retstr;
+ apr_size_t i, copied = 0;
+ int c;
+
+ /* At least one control character:
+ strlen - 1 (control) + \ + N + N + N + null . */
+ retstr = svn_stringbuf_create_ensure(strlen(path) + 4, pool);
+ for (i = 0; path[i]; i++)
+ {
+ c = (unsigned char)path[i];
+ if (! svn_ctype_iscntrl(c))
+ continue;
+
+ /* If we got here, we're looking at a character that isn't
+ supported by the (or at least, our) URI encoding scheme. We
+ need to escape this character. */
+
+ /* First things first, copy all the good stuff that we haven't
+ yet copied into our output buffer. */
+ if (i - copied)
+ svn_stringbuf_appendbytes(retstr, path + copied,
+ i - copied);
+
+ /* Make sure buffer is big enough for '\' 'N' 'N' 'N' (and NUL) */
+ svn_stringbuf_ensure(retstr, retstr->len + 5);
+ /*### The backslash separator doesn't work too great with Windows,
+ but it's what we'll use for consistency with invalid utf8
+ formatting (until someone has a better idea) */
+ apr_snprintf(retstr->data + retstr->len, 5, "\\%03o", (unsigned char)c);
+ retstr->len += 4;
+
+ /* Finally, update our copy counter. */
+ copied = i + 1;
+ }
+
+ /* If we didn't encode anything, we don't need to duplicate the string. */
+ if (retstr->len == 0)
+ return path;
+
+ /* Anything left to copy? */
+ if (i - copied)
+ svn_stringbuf_appendbytes(retstr, path + copied, i - copied);
+
+ /* retstr is null-terminated either by apr_snprintf or the svn_stringbuf
+ functions. */
+
+ return retstr->data;
+}
+
+/* Raise an error if PATH contains a newline because FSFS cannot handle
+ * such paths. See issue #4340. */
+static svn_error_t *
+check_newline(const char *path, apr_pool_t *pool)
+{
+ const char *c;
+
+ for (c = path; *c; c++)
+ {
+ if (*c == '\n')
+ return svn_error_createf(SVN_ERR_FS_PATH_SYNTAX, NULL,
+ _("Invalid control character '0x%02x' in path '%s'"),
+ (unsigned char)*c, illegal_path_escape(path, pool));
+ }
+
+ return SVN_NO_ERROR;
+}
/* Create a new directory named PATH in ROOT. The new directory has
no entries, and no properties. ROOT must be the root of a
@@ -1824,6 +1897,8 @@ fs_make_dir(svn_fs_root_t *root,
dag_node_t *sub_dir;
const char *txn_id = root->txn;
+ SVN_ERR(check_newline(path, pool));
+
SVN_ERR(open_path(&parent_path, root, path, open_path_last_optional,
txn_id, pool));
@@ -2086,6 +2161,8 @@ fs_copy(svn_fs_root_t *from_root,
const char *to_path,
apr_pool_t *pool)
{
+ SVN_ERR(check_newline(to_path, pool));
+
return copy_helper(from_root, from_path, to_root, to_path, TRUE, pool);
}
@@ -2176,6 +2253,8 @@ fs_make_file(svn_fs_root_t *root,
dag_node_t *child;
const char *txn_id = root->txn;
+ SVN_ERR(check_newline(path, pool));
+
SVN_ERR(open_path(&parent_path, root, path, open_path_last_optional,
txn_id, pool));
Modified: subversion/branches/1.6.x/subversion/tests/libsvn_fs/fs-test.c
URL: http://svn.apache.org/viewvc/subversion/branches/1.6.x/subversion/tests/libsvn_fs/fs-test.c?rev=1485299&r1=1485298&r2=1485299&view=diff
==============================================================================
--- subversion/branches/1.6.x/subversion/tests/libsvn_fs/fs-test.c (original)
+++ subversion/branches/1.6.x/subversion/tests/libsvn_fs/fs-test.c Wed May 22 17:35:00 2013
@@ -4987,6 +4987,68 @@ node_origin_rev(const char **msg,
return SVN_NO_ERROR;
}
+/* Issue 4340, "fs layer should reject filenames with trailing \n" */
+static svn_error_t *
+filename_trailing_newline(const char **msg,
+ svn_boolean_t msg_only,
+ svn_test_opts_t *opts,
+ apr_pool_t *pool)
+{
+ apr_pool_t *subpool = svn_pool_create(pool);
+ svn_fs_t *fs;
+ svn_fs_txn_t *txn;
+ svn_fs_root_t *txn_root, *root;
+ svn_revnum_t youngest_rev = 0;
+ svn_error_t *err;
+ svn_boolean_t allow_newlines;
+
+ /* Some filesystem implementations can handle newlines in filenames
+ * and can be white-listed here.
+ * Currently, only BDB supports \n in filenames. */
+ allow_newlines = (strcmp(opts->fs_type, "bdb") == 0);
+
+ *msg = "filenames with trailing \\n might be rejected";
+ if (msg_only)
+ return SVN_NO_ERROR;
+
+ SVN_ERR(svn_test__create_fs(&fs, "test-filename-trailing-newline",
+ opts, pool));
+
+ /* Revision 1: Add a directory /foo */
+ SVN_ERR(svn_fs_begin_txn(&txn, fs, youngest_rev, subpool));
+ SVN_ERR(svn_fs_txn_root(&txn_root, txn, subpool));
+ SVN_ERR(svn_fs_make_dir(txn_root, "/foo", subpool));
+ SVN_ERR(svn_fs_commit_txn(NULL, &youngest_rev, txn, subpool));
+ SVN_TEST_ASSERT(SVN_IS_VALID_REVNUM(youngest_rev));
+ svn_pool_clear(subpool);
+
+ /* Attempt to copy /foo to "/bar\n". This should fail on FSFS. */
+ SVN_ERR(svn_fs_begin_txn(&txn, fs, youngest_rev, subpool));
+ SVN_ERR(svn_fs_txn_root(&txn_root, txn, subpool));
+ SVN_ERR(svn_fs_revision_root(&root, fs, youngest_rev, subpool));
+ err = svn_fs_copy(root, "/foo", txn_root, "/bar\n", subpool);
+ if (allow_newlines)
+ SVN_TEST_ASSERT(err == SVN_NO_ERROR);
+ else
+ {
+ SVN_TEST_ASSERT(err && err->apr_err == SVN_ERR_FS_PATH_SYNTAX);
+ svn_error_clear(err);
+ }
+
+ /* Attempt to create a file /foo/baz\n. This should fail on FSFS. */
+ err = svn_fs_make_file(txn_root, "/foo/baz\n", subpool);
+ if (allow_newlines)
+ SVN_TEST_ASSERT(err == SVN_NO_ERROR);
+ else
+ {
+ SVN_TEST_ASSERT(err && err->apr_err == SVN_ERR_FS_PATH_SYNTAX);
+ svn_error_clear(err);
+ }
+
+ return SVN_NO_ERROR;
+}
+
+
/* ------------------------------------------------------------------------ */
/* The test table. */
@@ -5030,5 +5092,6 @@ struct svn_test_descriptor_t test_funcs[
SVN_TEST_PASS(set_uuid),
SVN_TEST_PASS(node_origin_rev),
SVN_TEST_PASS(small_file_integrity),
+ SVN_TEST_PASS(filename_trailing_newline),
SVN_TEST_NULL
};