workspace share path hardening (#95)

* resolve relative paths for root path
* block share paths that go outside of root path
* fix test using invalid share_path
* validate that root path is absolute

3 files changed, 51 insertions, 6 deletions

diff --git a/src/zen/cmds/workspaces_cmd.cpp b/src/zen/cmds/workspaces_cmd.cpp
index afdf5d7f5..d83439b0a 100644
--- a/src/zen/cmds/workspaces_cmd.cpp
+++ b/src/zen/cmds/workspaces_cmd.cpp
@@ -91,7 +91,7 @@ WorkspaceCommand::Run(const ZenCliOptions& GlobalOptions, int argc, char** argv)
 			ZEN_CONSOLE("Using generated workspace id from path '{}'", m_Path);
 		}
 
-		HttpClient::KeyValueMap Params{{"root_path", m_Path}};
+		HttpClient::KeyValueMap Params{{"root_path", std::filesystem::absolute(m_Path).string()}};
 		if (HttpClient::Response Result = Http.Put(fmt::format("/ws/{}", m_Id), Params))
 		{
 			ZEN_CONSOLE("{}. Id: {}", Result, Result.AsText());
@@ -276,7 +276,7 @@ WorkspaceShareCommand::Run(const ZenCliOptions& GlobalOptions, int argc, char**
 	{
 		if (!m_WorkspaceRoot.empty())
 		{
-			HttpClient::KeyValueMap Params{{"root_path", m_WorkspaceRoot}};
+			HttpClient::KeyValueMap Params{{"root_path", std::filesystem::absolute(m_WorkspaceRoot).string()}};
 			if (HttpClient::Response Result =
 					Http.Put(fmt::format("/ws/{}", m_WorkspaceId.empty() ? Oid::Zero.ToString() : m_WorkspaceId), Params))
 			{
diff --git a/src/zenserver-test/zenserver-test.cpp b/src/zenserver-test/zenserver-test.cpp
index 15f863002..15993f5c9 100644
--- a/src/zenserver-test/zenserver-test.cpp
+++ b/src/zenserver-test/zenserver-test.cpp
@@ -3383,10 +3383,10 @@ TEST_CASE("workspaces.lifetimes")
 		CHECK(Client.Put(fmt::format("/ws/{}", WorkspaceId), HttpClient::KeyValueMap{{"root_path", RootPath.string()}}).StatusCode ==
 			  HttpResponseCode::OK);
 
-		CHECK(Client.Put(fmt::format("/ws/{}/{}", WorkspaceId, ShareId), HttpClient::KeyValueMap{{"share_path", SharePath.string()}})
+		CHECK(Client.Put(fmt::format("/ws/{}/{}", WorkspaceId, ShareId), HttpClient::KeyValueMap{{"share_path", "shared_folder"}})
 				  .StatusCode == HttpResponseCode::Created);
 		CHECK(Client.Get(fmt::format("/ws/{}/{}", WorkspaceId, ShareId)).AsObject()["id"sv].AsObjectId() == ShareId);
-		CHECK(Client.Put(fmt::format("/ws/{}/{}", WorkspaceId, ShareId), HttpClient::KeyValueMap{{"share_path", SharePath.string()}})
+		CHECK(Client.Put(fmt::format("/ws/{}/{}", WorkspaceId, ShareId), HttpClient::KeyValueMap{{"share_path", "shared_folder"}})
 				  .StatusCode == HttpResponseCode::OK);
 	}
 
@@ -3445,8 +3445,8 @@ TEST_CASE("workspaces.share")
 	CHECK(Client.Get(fmt::format("/ws/{}", WorkspaceId)).AsObject()["id"sv].AsObjectId() == WorkspaceId);
 
 	Oid ShareId = Oid::NewOid();
-	CHECK(Client.Put(fmt::format("/ws/{}/{}", WorkspaceId, ShareId), HttpClient::KeyValueMap{{"share_path", SharePath.string()}})
-			  .StatusCode == HttpResponseCode::Created);
+	CHECK(Client.Put(fmt::format("/ws/{}/{}", WorkspaceId, ShareId), HttpClient::KeyValueMap{{"share_path", "shared_folder"}}).StatusCode ==
+		  HttpResponseCode::Created);
 	CHECK(Client.Get(fmt::format("/ws/{}/{}", WorkspaceId, ShareId)).AsObject()["id"sv].AsObjectId() == ShareId);
 
 	CHECK(Client.Get(fmt::format("/ws/{}/{}/files", WorkspaceId, ShareId)).AsObject()["files"sv].AsArrayView().Num() == 8);
diff --git a/src/zenstore/workspaces.cpp b/src/zenstore/workspaces.cpp
index 6d8f8970d..01f1cc55f 100644
--- a/src/zenstore/workspaces.cpp
+++ b/src/zenstore/workspaces.cpp
@@ -408,6 +408,13 @@ Workspaces::AddWorkspace(const WorkspaceConfiguration& Configuration)
 	{
 		return false;
 	}
+
+	const std::filesystem::path RootPath = Configuration.RootPath;
+	if (RootPath.is_relative())
+	{
+		throw std::invalid_argument(fmt::format("workspace root path '{}' is not an absolute path", RootPath));
+	}
+
 	m_Workspaces.insert(std::make_pair(Configuration.Id, NewWorkspace));
 	ZEN_INFO("Created workspace '{}' with root '{}'", Configuration.Id, Configuration.RootPath);
 	return true;
@@ -499,6 +506,16 @@ Workspaces::AddWorkspaceShare(const Oid&												   WorkspaceId,
 		}
 	}
 
+	const std::filesystem::path RootPath  = Workspace->GetConfig().RootPath;
+	const std::filesystem::path SharePath = Configuration.SharePath;
+
+	std::filesystem::path FullPath		  = std::filesystem::absolute(RootPath / SharePath);
+	std::filesystem::path VerifySharePath = std::filesystem::relative(FullPath, RootPath);
+	if (VerifySharePath != Configuration.SharePath || VerifySharePath.string().starts_with(".."))
+	{
+		throw std::invalid_argument(fmt::format("workspace share path '{}' is not valid for root path '{}'", SharePath, RootPath));
+	}
+
 	Ref<WorkspaceShare> NewShare(new WorkspaceShare(Configuration, {}, PathToIdCB));
 	{
 		RwLock::ExclusiveLockScope _(m_Lock);
@@ -883,6 +900,34 @@ TEST_CASE("workspaces.scanfolder")
 	});
 }
 
+TEST_CASE("workspace.share.paths")
+{
+	using namespace std::literals;
+
+	WorkerThreadPool WorkerPool(std::thread::hardware_concurrency());
+
+	ScopedTemporaryDirectory								TempDir;
+	std::filesystem::path									RootPath = TempDir.Path();
+	std::vector<std::pair<std::filesystem::path, IoBuffer>> Content	 = GenerateFolderContent(RootPath);
+	Workspaces												WS;
+	CHECK(WS.AddWorkspace({PathToId(RootPath), RootPath}));
+	CHECK(WS.AddWorkspaceShare(PathToId(RootPath),
+							   {PathToId("second_folder/child_in_second"), "second_folder/child_in_second"},
+							   [](const std::filesystem::path& Path) { return PathToId(Path); }));
+	CHECK_THROWS(WS.AddWorkspaceShare(PathToId(RootPath),
+									  {PathToId("../second_folder"), "../second_folder"},
+									  [](const std::filesystem::path& Path) { return PathToId(Path); }));
+	CHECK(WS.AddWorkspaceShare(PathToId(RootPath), {PathToId("."), "."}, [](const std::filesystem::path& Path) { return PathToId(Path); }));
+	CHECK_THROWS(
+		WS.AddWorkspaceShare(PathToId(RootPath), {PathToId(".."), ".."}, [](const std::filesystem::path& Path) { return PathToId(Path); }));
+	CHECK_THROWS(WS.AddWorkspaceShare(PathToId(RootPath),
+									  {PathToId("second_folder/../second_folder/../.."), "second_folder/../second_folder/../.."},
+									  [](const std::filesystem::path& Path) { return PathToId(Path); }));
+	CHECK_THROWS(WS.AddWorkspaceShare(PathToId(RootPath), {PathToId(RootPath), RootPath}, [](const std::filesystem::path& Path) {
+		return PathToId(Path);
+	}));
+}
+
 TEST_CASE("workspace.share.basic")
 {
 	using namespace std::literals;