Replace first level permission checks and path resolution/normalization

4 files changed, 106 insertions, 16 deletions

diff --git a/netfs/daemon/daemonVolume.cpp b/netfs/daemon/daemonVolume.cpp
index a84c2e1..2d67272 100644
--- a/netfs/daemon/daemonVolume.cpp
+++ b/netfs/daemon/daemonVolume.cpp
@@ -48,7 +48,7 @@ VolumeServer::access(const NetFS::ReqEnv & re, const std::string & path, Ice::In
 		// stat succeeded, file must exist
 		return 0;
 	}
-	mc.AssertRead(p.parent_path());
+	mc.AssertReadParent(p);
 	if (mode & R_OK && !mc.ReadableBy(s, mc.myu, mc.myg)) {
 		return EACCES;
 	}
@@ -67,7 +67,7 @@ VolumeServer::getattr(const NetFS::ReqEnv & re, const std::string & path, const
 	ModeCheck mc(re, root, userLookup, groupLookup);
 	struct stat s;
 	boost::filesystem::path p(resolvePath(path));
-	mc.AssertRead(p.parent_path());
+	mc.AssertReadParent(p);
 	if (::lstat(p.c_str(), &s) != 0) {
 		throw NetFS::SystemError(errno);
 	}
@@ -82,7 +82,7 @@ VolumeServer::mknod(const NetFS::ReqEnv & re, const std::string & path, Ice::Int
 	ModeCheck mc(re, root, userLookup, groupLookup);
 	errno = 0;
 	boost::filesystem::path p(resolvePath(path));
-	mc.AssertWrite(p.parent_path());
+	mc.AssertWriteParent(p);
 	if (::mknod(p.c_str(), mode, dev) != 0) {
 		throw NetFS::SystemError(errno);
 	}
@@ -94,7 +94,7 @@ VolumeServer::symlink(const NetFS::ReqEnv & re, const std::string & path1, const
 	ModeCheck mc(re, root, userLookup, groupLookup);
 	errno = 0;
 	boost::filesystem::path p(resolvePath(path2));
-	mc.AssertWrite(p.parent_path());
+	mc.AssertWriteParent(p);
 	if (::symlink(path1.c_str(), p.c_str()) != 0) {
 		throw NetFS::SystemError(errno);
 	}
@@ -127,13 +127,13 @@ VolumeServer::rename(const NetFS::ReqEnv & re, const std::string & from, const s
 	errno = 0;
 	boost::filesystem::path f(resolvePath(from));
 	boost::filesystem::path t(resolvePath(to));
-	mc.AssertWrite(f.parent_path());
+	mc.AssertWriteParent(f);
 	mc.AssertWrite(f);
 	if (boost::filesystem::is_directory(t)) {
 		mc.AssertWrite(t);
 	}
 	else {
-		mc.AssertWrite(t.parent_path());
+		mc.AssertWriteParent(t);
 	}
 	if (::rename(f.c_str(), t.c_str()) != 0) {
 		throw NetFS::SystemError(errno);
@@ -232,7 +232,7 @@ VolumeServer::unlink(const NetFS::ReqEnv & re, const std::string & path, const I
 	errno = 0;
 	boost::filesystem::path p(resolvePath(path));
 	mc.AssertWrite(p);
-	mc.AssertWrite(p.parent_path());
+	mc.AssertWriteParent(p);
 	if (::unlink(p.c_str()) != 0) {
 		throw NetFS::SystemError(errno);
 	}
@@ -261,7 +261,7 @@ VolumeServer::create(const NetFS::ReqEnv & re, const std::string & path, Ice::In
 	ModeCheck mc(re, root, userLookup, groupLookup);
 	errno = 0;
 	boost::filesystem::path p(resolvePath(path));
-	mc.AssertWrite(p.parent_path());
+	mc.AssertWriteParent(p);
 	int fd = ::open(p.c_str(), O_CREAT | flags, mode);
 	if (fd == -1) {
 		throw NetFS::SystemError(errno);
@@ -294,7 +294,7 @@ VolumeServer::mkdir(const NetFS::ReqEnv & re, const std::string & path, Ice::Int
 	ModeCheck mc(re, root, userLookup, groupLookup);
 	errno = 0;
 	boost::filesystem::path p(resolvePath(path));
-	mc.AssertWrite(p.parent_path());
+	mc.AssertWriteParent(p);
 	if (::mkdir(p.c_str(), mode) != 0) {
 		throw NetFS::SystemError(errno);
 	}
@@ -311,20 +311,38 @@ VolumeServer::rmdir(const NetFS::ReqEnv & re, const std::string & path, const Ic
 	errno = 0;
 	boost::filesystem::path p(resolvePath(path));
 	mc.AssertWrite(p);
-	mc.AssertWrite(p.parent_path());
+	mc.AssertWriteParent(p);
 	if (::rmdir(p.c_str()) != 0) {
 		throw NetFS::SystemError(errno);
 	}
 }
 
 boost::filesystem::path
-VolumeServer::resolvePath(const std::string & path) const
+normalizedAppend(boost::filesystem::path out, const boost::filesystem::path & in)
 {
-	Lock(lock);
-	auto p((root / path).normalize());
-	if (!boost::algorithm::starts_with(p.string(), root.string())) {
-		throw NetFS::SystemError(EPERM);	
+	unsigned int depth = 0;
+	for(auto e : in) {
+		if (e.empty() || e == "." || e == "/") {
+			continue;
+		}
+		else if (e == "..") {
+			if (depth == 0) {
+				throw NetFS::SystemError(EPERM);
+			}
+			out.remove_leaf();
+			depth -= 1;
+		}
+		else {
+			out /= e;
+			depth += 1;
+		}
 	}
-	return p;
+	return out;
+}
+
+boost::filesystem::path
+VolumeServer::resolvePath(const std::string & path) const
+{
+	return normalizedAppend(root, path);
 }
 
diff --git a/netfs/daemon/modeCheck.cpp b/netfs/daemon/modeCheck.cpp
index 8468f52..dd1464c 100644
--- a/netfs/daemon/modeCheck.cpp
+++ b/netfs/daemon/modeCheck.cpp
@@ -12,6 +12,14 @@ ModeCheck::ModeCheck(const NetFS::ReqEnv & re, const boost::filesystem::path & r
 }
 
 void
+ModeCheck::AssertReadParent(const boost::filesystem::path & p) const
+{
+	if (p != root) {
+		AssertRead(p.parent_path());
+	}
+}
+
+void
 ModeCheck::AssertRead(const boost::filesystem::path & p) const
 {
 	if (p != root) {
@@ -23,6 +31,17 @@ ModeCheck::AssertRead(const boost::filesystem::path & p) const
 }
 
 void
+ModeCheck::AssertWriteParent(const boost::filesystem::path & p) const
+{
+	if (p != root) {
+		AssertWrite(p.parent_path());
+	}
+	else {
+		throw NetFS::SystemError(EACCES);
+	}
+}
+
+void
 ModeCheck::AssertWrite(const boost::filesystem::path & p) const
 {
 	if (p != root) {
diff --git a/netfs/daemon/modeCheck.h b/netfs/daemon/modeCheck.h
index d43fa2f..6c3ee2c 100644
--- a/netfs/daemon/modeCheck.h
+++ b/netfs/daemon/modeCheck.h
@@ -10,7 +10,9 @@ class ModeCheck {
 	public:
 		ModeCheck(const NetFS::ReqEnv & re, const boost::filesystem::path &, const EntCache<User> &, const EntCache<Group> &);
 
+		void AssertReadParent(const boost::filesystem::path &) const;
 		void AssertRead(const boost::filesystem::path &) const;
+		void AssertWriteParent(const boost::filesystem::path &) const;
 		void AssertWrite(const boost::filesystem::path &) const;
 
 		const uid_t myu;
diff --git a/netfs/unittests/testCore.cpp b/netfs/unittests/testCore.cpp
index b56e11c..c3b925b 100644
--- a/netfs/unittests/testCore.cpp
+++ b/netfs/unittests/testCore.cpp
@@ -4,10 +4,24 @@
 #include "mockFuse.h"
 #include <definedDirs.h>
 #include <boost/filesystem/path.hpp>
+#include <ostream>
 
 const std::string testEndpoint("tcp -h localhost -p 12012");
 const std::string testUri("tcp://localhost:12012/testvol");
 
+bool
+operator==(const struct stat & a, const struct stat & b)
+{
+	return (a.st_dev == b.st_dev && a.st_ino == b.st_ino);
+}
+
+namespace std {
+	ostream & operator<<(ostream & s, const struct stat & ss)
+	{
+		return s << "dev: " << ss.st_dev << " inode: " << ss.st_ino;
+	}
+}
+
 class Core {
 	public:
 		Core(const std::string & daemonConfig = "defaultDaemon.xml", const std::string & fuseConfig = "defaultFuse.xml") :
@@ -48,6 +62,43 @@ BOOST_AUTO_TEST_CASE ( clientInitialised )
 	BOOST_REQUIRE_EQUAL(0, fuse->statfs("/",  &s));
 }
 
+BOOST_AUTO_TEST_CASE( testNavigation )
+{
+	const auto testExport = stringbf("testExport-%d", getpid());
+	struct stat attr, rootattr, insideattr;
+	memset(&attr, 0, sizeof(attr));
+	memset(&rootattr, 0, sizeof(rootattr));
+	memset(&insideattr, 0, sizeof(insideattr));
+	struct fuse_file_info fi;
+	memset(&fi, 0, sizeof(fi));
+	int fd = fuse->create("/inside", 0666, &fi);
+	BOOST_REQUIRE(fd >= 0);
+	fuse->release("/inside", &fi);
+	BOOST_REQUIRE(boost::filesystem::exists(binDir / testExport / "inside"));
+	BOOST_REQUIRE_EQUAL(lstat((binDir / testExport).c_str(), &rootattr), 0);
+	BOOST_REQUIRE_EQUAL(lstat((binDir / testExport / "inside").c_str(), &insideattr), 0);
+	BOOST_REQUIRE_EQUAL(0, fuse->getattr("/", &attr));
+	BOOST_REQUIRE_EQUAL(attr, rootattr);
+	BOOST_REQUIRE_EQUAL(0, fuse->getattr(".", &attr));
+	BOOST_REQUIRE_EQUAL(attr, rootattr);
+	BOOST_REQUIRE_EQUAL(0, fuse->getattr("", &attr));
+	BOOST_REQUIRE_EQUAL(attr, rootattr);
+	BOOST_REQUIRE_EQUAL(0, fuse->getattr("/.", &attr));
+	BOOST_REQUIRE_EQUAL(attr, rootattr);
+	BOOST_REQUIRE_EQUAL(0, fuse->getattr("/inside/..", &attr));
+	BOOST_REQUIRE_EQUAL(attr, rootattr);
+	BOOST_REQUIRE_EQUAL(0, fuse->getattr("//inside/./..", &attr));
+	BOOST_REQUIRE_EQUAL(attr, rootattr);
+	BOOST_REQUIRE_EQUAL(0, fuse->getattr("/inside", &attr));
+	BOOST_REQUIRE_EQUAL(attr, insideattr);
+	BOOST_REQUIRE_EQUAL(0, fuse->getattr("/inside/", &attr));
+	BOOST_REQUIRE_EQUAL(attr, insideattr);
+	BOOST_REQUIRE_EQUAL(0, fuse->getattr("/anything/../inside", &attr));
+	BOOST_REQUIRE_EQUAL(attr, insideattr);
+	BOOST_REQUIRE_EQUAL(0, fuse->getattr("/inside/../inside", &attr));
+	BOOST_REQUIRE_EQUAL(attr, insideattr);
+}
+
 BOOST_AUTO_TEST_CASE( testSandboxing )
 {
 	const auto testExport = stringbf("testExport-%d", getpid());