From aee23409dad4ba368752a4c68e27d75b79090c48 Mon Sep 17 00:00:00 2001
From: Markus Rudy <mr@edgeless.systems>
Date: Fri, 19 Jul 2024 16:48:35 +0200
Subject: [PATCH] genpolicy: harden CopyFileRequest checks

CopyFile is invoked by the host's FileSystemShare.ShareFile function,
which puts all files into directories with a common pattern. Copying
files anywhere else is dangerous and must be prevented. Thus, we check
that the target path prefix matches the expected directory pattern of
ShareFile, and that this directory is not escaped by .. traversal.

Signed-off-by: Markus Rudy <mr@edgeless.systems>
---
 src/tools/genpolicy/genpolicy-settings.json   |  2 +-
 src/tools/genpolicy/rules.rego                | 15 +++++-
 .../tests/testdata/copyfile/testcases.json    | 49 +++++++++++++++++++
 3 files changed, 63 insertions(+), 3 deletions(-)

diff --git a/src/tools/genpolicy/genpolicy-settings.json b/src/tools/genpolicy/genpolicy-settings.json
index 309e6a0767..aa22ec882c 100644
--- a/src/tools/genpolicy/genpolicy-settings.json
+++ b/src/tools/genpolicy/genpolicy-settings.json
@@ -296,7 +296,7 @@
             ]
         },
         "CopyFileRequest": [
-            "^$(cpath)/"
+            "$(sfprefix)"
         ],
         "ExecProcessRequest": {
             "allowed_commands": [],
diff --git a/src/tools/genpolicy/rules.rego b/src/tools/genpolicy/rules.rego
index 07bb85a884..3db3ed3dc7 100644
--- a/src/tools/genpolicy/rules.rego
+++ b/src/tools/genpolicy/rules.rego
@@ -1092,12 +1092,23 @@ match_caps(p_caps, i_caps) {
 }
 
 ######################################################################
+
+check_directory_traversal(i_path) {
+    not regex.match("(^|/)..($|/)", i_path)
+}
+
 CopyFileRequest {
     print("CopyFileRequest: input.path =", input.path)
 
+    check_directory_traversal(input.path)
+
     some regex1 in policy_data.request_defaults.CopyFileRequest
-    regex2 := replace(regex1, "$(cpath)", policy_data.common.cpath)
-    regex.match(regex2, input.path)
+    regex2 := replace(regex1, "$(sfprefix)", policy_data.common.sfprefix)
+    regex3 := replace(regex2, "$(cpath)", policy_data.common.mount_source_cpath)
+    regex4 := replace(regex3, "$(bundle-id)", "[a-z0-9]{64}")
+    print("CopyFileRequest: regex4 =", regex4)
+
+    regex.match(regex4, input.path)
 
     print("CopyFileRequest: true")
 }
diff --git a/src/tools/genpolicy/tests/testdata/copyfile/testcases.json b/src/tools/genpolicy/tests/testdata/copyfile/testcases.json
index 0d420d81cf..d6b56f7a19 100644
--- a/src/tools/genpolicy/tests/testdata/copyfile/testcases.json
+++ b/src/tools/genpolicy/tests/testdata/copyfile/testcases.json
@@ -6,11 +6,60 @@
       "path": "/run/kata-containers/shared/containers/81e5f43bc8599c5661e66f959ac28df5bfb30da23c5d583f2dcc6f9e0c5186dc-ce23cfeb91e75aaa-resolv.conf"
     }
   },
+  {
+    "description": "a dirname can have trailing dots",
+    "allowed": true,
+    "request": {
+      "path": "/run/kata-containers/shared/containers/81e5f43bc8599c5661e66f959ac28df5bfb30da23c5d583f2dcc6f9e0c5186dc-ce23cfeb91e75aaa-foo../bar"
+    }
+  },
   {
     "description": "attempt to copy outside of container root",
     "allowed": false,
     "request": {
       "path": "/etc/ssl/cert.pem"
     }
+  },
+  {
+    "description": "attempt to write into container root",
+    "allowed": false,
+    "request": {
+      "path": "/run/kata-containers/shared/containers/81e5f43bc8599c5661e66f959ac28df5bfb30da23c5d583f2dcc6f9e0c5186dc/rootfs/bin/sh"
+    }
+  },
+  {
+    "description": "attempt to write into container root - guest pull",
+    "allowed": false,
+    "request": {
+      "path": "/run/kata-containers/81e5f43bc8599c5661e66f959ac28df5bfb30da23c5d583f2dcc6f9e0c5186dc/rootfs/bin/sh"
+    }
+  },
+  {
+    "description": "attempted directory traversal",
+    "allowed": false,
+    "request": {
+      "path": "/run/kata-containers/shared/containers/81e5f43bc8599c5661e66f959ac28df5bfb30da23c5d583f2dcc6f9e0c5186dc-ce23cfeb91e75aaa-foo/../../../../../etc/ssl/cert.pem"
+    }
+  },
+  {
+    "description": "attempted directory traversal - parent directory",
+    "allowed": false,
+    "request": {
+      "path": "/run/kata-containers/shared/containers/81e5f43bc8599c5661e66f959ac28df5bfb30da23c5d583f2dcc6f9e0c5186dc-ce23cfeb91e75aaa-foo/.."
+    }
+  },
+  {
+    "description": "relative path",
+    "allowed": false,
+    "request": {
+      "path": "etc/ssl/cert.pem"
+    }
+  },
+  {
+    "description": "relative path - parent directory",
+    "allowed": false,
+    "request": {
+      "path": ".."
+    }
   }
 ]