From beb1c55971272db00353259ab07b27be6399453a Mon Sep 17 00:00:00 2001 From: huynhtrungcsc <250898624+huynhtrungcsc@users.noreply.github.com> Date: Sat, 4 Jul 2026 09:02:49 +0700 Subject: [PATCH] Restrict untrusted runner file RPC paths --- .../bot/untrusted_runner/file_impl.py | 46 +++++++++++- .../bot/untrusted_runner/file_impl_test.py | 75 +++++++++++++++++++ 2 files changed, 119 insertions(+), 2 deletions(-) diff --git a/src/clusterfuzz/_internal/bot/untrusted_runner/file_impl.py b/src/clusterfuzz/_internal/bot/untrusted_runner/file_impl.py index 17cefff4489..273c017ca0e 100644 --- a/src/clusterfuzz/_internal/bot/untrusted_runner/file_impl.py +++ b/src/clusterfuzz/_internal/bot/untrusted_runner/file_impl.py @@ -17,6 +17,7 @@ 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 @@ -24,14 +25,47 @@ # 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) @@ -39,6 +73,9 @@ def remove_directory(request, _): 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) diff --git a/src/clusterfuzz/_internal/tests/core/bot/untrusted_runner/file_impl_test.py b/src/clusterfuzz/_internal/tests/core/bot/untrusted_runner/file_impl_test.py index 2f44bdf5028..ed920f949cf 100644 --- a/src/clusterfuzz/_internal/tests/core/bot/untrusted_runner/file_impl_test.py +++ b/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')])