From a10dec76b856876039ce80ec0f26c9a5141bcab2 Mon Sep 17 00:00:00 2001 From: Dan Goodliffe Date: Sun, 14 Feb 2016 19:06:41 +0000 Subject: Replace first level permission checks and path resolution/normalization --- netfs/daemon/daemonVolume.cpp | 50 ++++++++++++++++++++++++++++-------------- netfs/daemon/modeCheck.cpp | 19 ++++++++++++++++ netfs/daemon/modeCheck.h | 2 ++ netfs/unittests/testCore.cpp | 51 +++++++++++++++++++++++++++++++++++++++++++ 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 @@ -11,6 +11,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 { @@ -22,6 +30,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 { 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 &, const EntCache &); + 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 #include +#include 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()); -- cgit v1.2.3