#
#
# patch "ChangeLog"
# from [825937415d488b8e9da543988c0893017ae8104c]
# to [2ea5aca2a64eafecb09594b4155c91ea2c1b03d9]
#
# patch "paths.cc"
# from [548609dee53e3a5f45191ff2debfab932a6864be]
# to [1cd1c1b1312df4a3e80accf2c08f672b3a0a50d7]
#
============================================================
--- ChangeLog 825937415d488b8e9da543988c0893017ae8104c
+++ ChangeLog 2ea5aca2a64eafecb09594b4155c91ea2c1b03d9
@@ -1,3 +1,10 @@
+2006-03-28 Nathaniel Smith
+
+ * paths.cc: s/MT/_MTN/ everywhere.
+ (in_bookkeeping_dir): Also apply the security kluge from 0.25.2,
+ where we define the bookkeeping dir case-insensitively.
+ Add tests for this.
+
2006-03-27 Nathaniel Smith
* database.cc (check_format): Use .empty() instead of comparing
============================================================
--- paths.cc 548609dee53e3a5f45191ff2debfab932a6864be
+++ paths.cc 1cd1c1b1312df4a3e80accf2c08f672b3a0a50d7
@@ -68,15 +68,15 @@
static access_tracker initial_abs_path;
// initial_rel_path is for interpreting external file_path's
// for now we just make it an fs::path for convenience; we used to make it a
-// file_path, but then you can't run monotone from inside the MT/ dir (even
-// when referring to files outside the MT/ dir).
+// file_path, but then you can't run monotone from inside the _MTN/ dir (even
+// when referring to files outside the _MTN/ dir).
static access_tracker initial_rel_path;
// working_root is for converting file_path's and bookkeeping_path's to
// system_path's.
static access_tracker working_root;
-bookkeeping_path const bookkeeping_root("MT");
-path_component const bookkeeping_root_component("MT");
+bookkeeping_path const bookkeeping_root("_MTN");
+path_component const bookkeeping_root_component("_MTN");
void
save_initial_path()
@@ -157,10 +157,29 @@
return true;
}
+// This function considers _MTN, _MTn, _MtN, _mtn etc. to all be bookkeeping
+// paths, because on case insensitive filesystems, files put in any of them
+// may end up in _MTN instead. This allows arbitrary code execution. A
+// better solution would be to fix this in the working directory writing code
+// -- this prevents all-unix projects from naming things "mt", which is a bit
+// rude -- but as a temporary security kluge it works.
static inline bool
in_bookkeeping_dir(std::string const & path)
{
- return path == "MT" || (path.size() >= 3 && (path.substr(0, 3) == "MT/"));
+ if (path.size() == 0 || (path[0] != '_'))
+ return false;
+ if (path.size() == 1 || (path[0] != 'M' && path[0] != 'm'))
+ return false;
+ if (path.size() == 2 || (path[1] != 'T' && path[1] != 't'))
+ return false;
+ if (path.size() == 3 || (path[1] != 'N' && path[1] != 'n'))
+ return false;
+ // if we've gotten here, the first three letters are _, M, T, and N, in
+ // either upper or lower case. So if that is the whole path, or else if it
+ // continues but the next character is /, then this is a bookkeeping path.
+ if (path.size() == 2 || (path[2] == '/'))
+ return true;
+ return false;
}
static inline bool
@@ -539,7 +558,7 @@
return false;
}
- // check for MT/. and MT/.. to see if mt dir is readable
+ // check for _MTN/. and _MTN/.. to see if mt dir is readable
if (!fs::exists(check / ".") || !fs::exists(check / ".."))
{
L(FL("problems with '%s' (missing '.' or '..')\n") % check.string());
@@ -583,7 +602,8 @@
"foo//bar",
"foo/../bar",
"../bar",
- "MT/blah",
+ "_MTN",
+ "_MTN/blah",
"foo/bar/",
"foo/./bar",
"./foo",
@@ -592,6 +612,22 @@
"c:\\foo",
"c:foo",
"c:/foo",
+ // some baddies made bad by a security kluge --
+ // see the comment in in_bookkeeping_dir
+ "_mtn",
+ "_mtN",
+ "_mTn",
+ "_Mtn",
+ "_MTn",
+ "_MtN",
+ "_mTN",
+ "_mtn/foo",
+ "_mtN/foo",
+ "_mTn/foo",
+ "_Mtn/foo",
+ "_MTn/foo",
+ "_MtN/foo",
+ "_mTN/foo",
0 };
initial_rel_path.unset();
initial_rel_path.set(fs::path(), true);
@@ -619,7 +655,7 @@
"foo/with,address@hidden/bar",
".foo/bar",
"..foo/bar",
- "MTfoo/bar",
+ "_MTNfoo/bar",
"foo:bar",
0 };
@@ -677,8 +713,8 @@
char const * baddies[] = {"/foo",
"../bar",
- "MT/blah",
- "MT",
+ "_MTN/blah",
+ "_MTN",
"//blah",
"\\foo",
"..",
@@ -686,6 +722,22 @@
"c:foo",
"c:/foo",
"",
+ // some baddies made bad by a security kluge --
+ // see the comment in in_bookkeeping_dir
+ "_mtn",
+ "_mtN",
+ "_mTn",
+ "_Mtn",
+ "_MTn",
+ "_MtN",
+ "_mTN",
+ "_mtn/foo",
+ "_mtN/foo",
+ "_mTn/foo",
+ "_Mtn/foo",
+ "_MTn/foo",
+ "_MtN/foo",
+ "_mTN/foo",
0 };
for (char const ** c = baddies; *c; ++c)
{
@@ -721,10 +773,10 @@
initial_rel_path.unset();
}
-static void test_file_path_external_prefix_MT()
+static void test_file_path_external_prefix__MTN()
{
initial_rel_path.unset();
- initial_rel_path.set(fs::path("MT"), true);
+ initial_rel_path.set(fs::path("_MTN"), true);
BOOST_CHECK_THROW(file_path_external(utf8("foo")), informative_failure);
BOOST_CHECK_THROW(file_path_external(utf8(".")), informative_failure);
@@ -741,8 +793,8 @@
char const * baddies[] = {"/foo",
"../../../bar",
"../../..",
- "../../MT",
- "../../MT/foo",
+ "../../_MTN",
+ "../../_MTN/foo",
"//blah",
"\\foo",
"c:\\foo",
@@ -751,6 +803,22 @@
"c:/foo",
#endif
"",
+ // some baddies made bad by a security kluge --
+ // see the comment in in_bookkeeping_dir
+ "../../_mtn",
+ "../../_mtN",
+ "../../_mTn",
+ "../../_Mtn",
+ "../../_MTn",
+ "../../_MtN",
+ "../../_mTN",
+ "../../_mtn/foo",
+ "../../_mtN/foo",
+ "../../_mTn/foo",
+ "../../_Mtn/foo",
+ "../../_MTn/foo",
+ "../../_MtN/foo",
+ "../../_mTN/foo",
0 };
for (char const ** c = baddies; *c; ++c)
{
@@ -785,8 +853,8 @@
check_fp_normalizes_to("../foo", "a/foo");
check_fp_normalizes_to("..", "a");
check_fp_normalizes_to("../..", "");
- check_fp_normalizes_to("MT/foo", "a/b/MT/foo");
- check_fp_normalizes_to("MT", "a/b/MT");
+ check_fp_normalizes_to("_MTN/foo", "a/b/_MTN/foo");
+ check_fp_normalizes_to("_MTN", "a/b/_MTN");
#ifndef WIN32
check_fp_normalizes_to("c:foo", "a/b/c:foo");
check_fp_normalizes_to("c:/foo", "a/b/c:/foo");
@@ -851,18 +919,34 @@
// Make sure that we can't use joining to create a path into the bookkeeping
// dir
split_path split_mt1, split_mt2;
- file_path_internal("foo/MT").split(split_mt1);
+ file_path_internal("foo/_MTN").split(split_mt1);
BOOST_CHECK(split_mt1.size() == 3);
I(split_mt1[2] == bookkeeping_root_component);
split_mt2.push_back(the_null_component);
split_mt2.push_back(split_mt1[2]);
- // split_mt2 now contains the component "MT"
+ // split_mt2 now contains the component "_MTN"
BOOST_CHECK_THROW(file_path(split_mt2) == file_path(), std::logic_error);
split_mt2.push_back(split_mt1[1]);
- // split_mt2 now contains the components "MT", "foo" in that order
+ // split_mt2 now contains the components "_MTN", "foo" in that order
// this comparison tricks the compiler into not completely eliminating this
// code as dead...
BOOST_CHECK_THROW(file_path(split_mt2) == file_path(), std::logic_error);
+
+ // and make sure it fails for the klugy security cases -- see comments on
+ // in_bookkeeping_dir
+ split_path split_mt1, split_mt2;
+ file_path_internal("foo/_mTn").split(split_mt1);
+ BOOST_CHECK(split_mt1.size() == 3);
+ I(split_mt1[2] == bookkeeping_root_component);
+ split_mt2.push_back(the_null_component);
+ split_mt2.push_back(split_mt1[2]);
+ // split_mt2 now contains the component "_mTn"
+ BOOST_CHECK_THROW(file_path(split_mt2) == file_path(), std::logic_error);
+ split_mt2.push_back(split_mt1[1]);
+ // split_mt2 now contains the components "_mTn", "foo" in that order
+ // this comparison tricks the compiler into not completely eliminating this
+ // code as dead...
+ BOOST_CHECK_THROW(file_path(split_mt2) == file_path(), std::logic_error);
}
static void check_bk_normalizes_to(char * before, char * after)
@@ -901,10 +985,10 @@
BOOST_CHECK_THROW(bookkeeping_path(tmp_path_string.assign("foo/bar")), std::logic_error);
BOOST_CHECK_THROW(bookkeeping_path(tmp_path_string.assign("a")), std::logic_error);
- check_bk_normalizes_to("a", "MT/a");
- check_bk_normalizes_to("foo", "MT/foo");
- check_bk_normalizes_to("foo/bar", "MT/foo/bar");
- check_bk_normalizes_to("foo/bar/baz", "MT/foo/bar/baz");
+ check_bk_normalizes_to("a", "_MTN/a");
+ check_bk_normalizes_to("foo", "_MTN/foo");
+ check_bk_normalizes_to("foo/bar", "_MTN/foo/bar");
+ check_bk_normalizes_to("foo/bar/baz", "_MTN/foo/bar/baz");
}
static void check_system_normalizes_to(char * before, char * after)
@@ -942,7 +1026,7 @@
// /work$ cd newdir
// /work/newdir$ monotone setup --db=../foo.db
// Now they have either "/work/foo.db" or "/work/newdir/../foo.db" in
- // MT/options
+ // _MTN/options
// /work/newdir$ cd ..
// /work$ mv newdir newerdir # better name
// Oops, now, if we stored the version with ..'s in, this workspace
@@ -992,10 +1076,10 @@
file_path a_file_path;
BOOST_CHECK(system_path(a_file_path).as_external()
== "/working/root");
- BOOST_CHECK(system_path(bookkeeping_path("MT/foo/bar")).as_internal()
- == "/working/root/MT/foo/bar");
+ BOOST_CHECK(system_path(bookkeeping_path("_MTN/foo/bar")).as_internal()
+ == "/working/root/_MTN/foo/bar");
BOOST_CHECK(system_path(bookkeeping_root).as_internal()
- == "/working/root/MT");
+ == "/working/root/_MTN");
initial_abs_path.unset();
working_root.unset();
initial_rel_path.unset();
@@ -1065,7 +1149,7 @@
suite->add(BOOST_TEST_CASE(&test_null_name));
suite->add(BOOST_TEST_CASE(&test_file_path_internal));
suite->add(BOOST_TEST_CASE(&test_file_path_external_null_prefix));
- suite->add(BOOST_TEST_CASE(&test_file_path_external_prefix_MT));
+ suite->add(BOOST_TEST_CASE(&test_file_path_external_prefix__MTN));
suite->add(BOOST_TEST_CASE(&test_file_path_external_prefix_a_b));
suite->add(BOOST_TEST_CASE(&test_split_join));
suite->add(BOOST_TEST_CASE(&test_bookkeeping_path));