Restrict untrusted runner file RPC paths

Author: DORIME17

src/clusterfuzz/_internal/bot/untrusted_runner/file_impl.py

@@ -17,28 +17,65 @@
 
 from clusterfuzz._internal.bot.fuzzers import utils as fuzzers_utils
 from clusterfuzz._internal.protos import untrusted_runner_pb2
+from clusterfuzz._internal.system import environment
 from clusterfuzz._internal.system import shell
 
 from . import file_utils
 
 # pylint: disable=no-member
 
 
+def _allowed_roots():
+  """Return worker filesystem roots that file RPCs may access."""
+  roots = []
+  for env_var in ('WORKER_ROOT_DIR', 'WORKER_BOT_TMPDIR'):
+    root = environment.get_value(env_var)
+    if root:
+      roots.append(os.path.realpath(root))
+
+  return roots
+
+
+def _is_allowed_path(path):
+  """Check whether path stays inside the worker-owned filesystem roots."""
+  if not path:
+    return False
+
+  roots = _allowed_roots()
+  if not roots:
+    return True
+
+  try:
+    real_path = os.path.realpath(path)
+    return any(os.path.commonpath([root, real_path]) == root for root in roots)
+  except ValueError:
+    return False
+
+
 def create_directory(request, _):
   """Create a directory."""
+  if not _is_allowed_path(request.path):
+    return untrusted_runner_pb2.CreateDirectoryResponse(result=False)
+
   result = shell.create_directory(request.path, request.create_intermediates)
   return untrusted_runner_pb2.CreateDirectoryResponse(result=result)
 
 
 def remove_directory(request, _):
   """Remove a directory."""
+  if not _is_allowed_path(request.path):
+    return untrusted_runner_pb2.RemoveDirectoryResponse(result=False)
+
   result = shell.remove_directory(request.path, request.recreate)
   return untrusted_runner_pb2.RemoveDirectoryResponse(result=result)
 
 
 def list_files(request, _):
   """List files."""
   file_paths = []
+  if not _is_allowed_path(request.path):
+    return untrusted_runner_pb2.ListFilesResponse(file_paths=file_paths)
+
   if request.recursive:
     for root, _, files in shell.walk(request.path):
       for filename in files:
@@ -54,6 +91,8 @@ def copy_file_to_worker(request_iterator, context):
   """Copy file from host to worker."""
   metadata = dict(context.invocation_metadata())
   path = metadata['path-bin'].decode('utf-8')
+  if not _is_allowed_path(path):
+    return untrusted_runner_pb2.CopyFileToResponse(result=False)
 
   # Create intermediate directories if needed.
   directory = os.path.dirname(path)
@@ -74,7 +113,7 @@ def copy_file_to_worker(request_iterator, context):
 def copy_file_from_worker(request, context):
   """Copy file from worker to host."""
   path = request.path
-  if not os.path.isfile(path):
+  if not _is_allowed_path(path) or not os.path.isfile(path):
     context.set_trailing_metadata([('result', 'invalid-path')])
     return
 
@@ -85,7 +124,7 @@ def copy_file_from_worker(request, context):
 
 def stat(request, _):
   """Stat a path."""
-  if not os.path.exists(request.path):
+  if not _is_allowed_path(request.path) or not os.path.exists(request.path):
     return untrusted_runner_pb2.StatResponse(result=False)
 
   stat_result = os.stat(request.path)
@@ -100,6 +139,9 @@ def stat(request, _):
 
 def get_fuzz_targets(request, _):
   """Get list of fuzz targets."""
+  if not _is_allowed_path(request.path):
+    return untrusted_runner_pb2.GetFuzzTargetsResponse(fuzz_target_paths=[])
+
   fuzz_target_paths = fuzzers_utils.get_fuzz_targets_local(request.path)
   return untrusted_runner_pb2.GetFuzzTargetsResponse(
       fuzz_target_paths=fuzz_target_paths)

src/clusterfuzz/_internal/tests/core/bot/untrusted_runner/file_impl_test.py

@@ -184,3 +184,78 @@ def test_copy_file_from_worker_failed(self):
     self.assertEqual(0, len(list(response)))
     context.set_trailing_metadata.assert_called_with([('result',
                                                        'invalid-path')])
+
+  def test_rejects_paths_outside_worker_roots(self):
+    """Test file_impl rejects paths outside worker-owned roots."""
+    os.makedirs('/worker/bot_tmp')
+    os.makedirs('/secret')
+    self.fs.create_file('/secret/file', contents='secret')
+
+    with mock.patch.dict(
+        os.environ, {
+            'WORKER_ROOT_DIR': '/worker',
+            'WORKER_BOT_TMPDIR': '/worker/bot_tmp',
+        }):
+      response = file_impl.create_directory(
+          untrusted_runner_pb2.CreateDirectoryRequest(
+              path='/secret/new_dir', create_intermediates=True), None)
+      self.assertFalse(response.result)
+      self.assertFalse(os.path.exists('/secret/new_dir'))
+
+      response = file_impl.remove_directory(
+          untrusted_runner_pb2.RemoveDirectoryRequest(
+              path='/secret', recreate=False), None)
+      self.assertFalse(response.result)
+      self.assertTrue(os.path.isdir('/secret'))
+
+      response = file_impl.list_files(
+          untrusted_runner_pb2.ListFilesRequest(path='/secret'), None)
+      self.assertEqual([], response.file_paths)
+
+      response = file_impl.stat(
+          untrusted_runner_pb2.StatRequest(path='/secret/file'), None)
+      self.assertFalse(response.result)
+
+      context = mock.MagicMock()
+      context.invocation_metadata.return_value = (('path-bin',
+                                                   b'/secret/out'),)
+      response = file_impl.copy_file_to_worker(
+          (untrusted_runner_pb2.FileChunk(data=b'A'),), context)
+      self.assertFalse(response.result)
+      self.assertFalse(os.path.exists('/secret/out'))
+
+      context = mock.MagicMock()
+      response = file_impl.copy_file_from_worker(
+          untrusted_runner_pb2.CopyFileFromRequest(path='/secret/file'),
+          context)
+      self.assertEqual([], list(response))
+      context.set_trailing_metadata.assert_called_with([('result',
+                                                         'invalid-path')])
+
+      response = file_impl.get_fuzz_targets(
+          untrusted_runner_pb2.GetFuzzTargetsRequest(path='/secret'), None)
+      self.assertEqual([], response.fuzz_target_paths)
+
+  def test_rejects_worker_root_symlink_escape(self):
+    """Test file_impl rejects worker-root paths resolving outside the root."""
+    os.makedirs('/worker')
+    os.makedirs('/secret')
+    self.fs.create_file('/secret/file', contents='secret')
+    os.symlink('/secret', '/worker/link')
+
+    with mock.patch.dict(os.environ, {'WORKER_ROOT_DIR': '/worker'}):
+      context = mock.MagicMock()
+      context.invocation_metadata.return_value = (('path-bin',
+                                                   b'/worker/link/out'),)
+      response = file_impl.copy_file_to_worker(
+          (untrusted_runner_pb2.FileChunk(data=b'A'),), context)
+      self.assertFalse(response.result)
+      self.assertFalse(os.path.exists('/secret/out'))
+
+      context = mock.MagicMock()
+      response = file_impl.copy_file_from_worker(
+          untrusted_runner_pb2.CopyFileFromRequest(path='/worker/link/file'),
+          context)
+      self.assertEqual([], list(response))
+      context.set_trailing_metadata.assert_called_with([('result',
+                                                         'invalid-path')])