From 8c63de35103d2f419bd9f9cf76a9217b1c8b7df0 Mon Sep 17 00:00:00 2001 From: herdiyana256 Date: Sat, 25 Jul 2026 23:23:04 +0700 Subject: [PATCH] Require testcase access in issue_redirector before redirecting GET /issue/ looked up the testcase and redirected straight to its issue tracker URL with no access check at all, not even the general access.has_access() used elsewhere -- this route has no auth decorator and helpers.get_testcase() is a plain datastore fetch with no ownership or security_flag check. testcase_id is a small sequential integer, so this let anyone, unauthenticated, enumerate testcases and learn the associated issue tracker URL for each one, including security-flagged/embargoed testcases that access.can_user_access_testcase() is meant to gate everywhere else in the app. Added a test confirming a caller without access gets a 403 instead of being redirected. --- src/appengine/handlers/issue_redirector.py | 4 ++++ .../appengine/handlers/issue_redirector_test.py | 16 ++++++++++++++++ 2 files changed, 20 insertions(+) diff --git a/src/appengine/handlers/issue_redirector.py b/src/appengine/handlers/issue_redirector.py index f66adc80fcb..7c987e408fc 100644 --- a/src/appengine/handlers/issue_redirector.py +++ b/src/appengine/handlers/issue_redirector.py @@ -16,6 +16,7 @@ from clusterfuzz._internal.issue_management import issue_tracker_utils from handlers import base_handler +from libs import access from libs import helpers @@ -25,6 +26,9 @@ class Handler(base_handler.Handler): def get(self, testcase_id=None): """Redirect user to the correct URL.""" testcase = helpers.get_testcase(testcase_id) + if not access.can_user_access_testcase(testcase): + raise helpers.AccessDeniedError() + issue_url = helpers.get_or_exit( lambda: issue_tracker_utils.get_issue_url(testcase), 'Issue tracker for testcase (id=%s) is not found.' % testcase_id, diff --git a/src/clusterfuzz/_internal/tests/appengine/handlers/issue_redirector_test.py b/src/clusterfuzz/_internal/tests/appengine/handlers/issue_redirector_test.py index b6a36d6cab0..df0939d6a7a 100644 --- a/src/clusterfuzz/_internal/tests/appengine/handlers/issue_redirector_test.py +++ b/src/clusterfuzz/_internal/tests/appengine/handlers/issue_redirector_test.py @@ -29,9 +29,11 @@ def setUp(self): test_helpers.patch(self, [ 'clusterfuzz._internal.issue_management.issue_tracker_utils.get_issue_url', 'libs.helpers.get_testcase', + 'libs.access.can_user_access_testcase', 'clusterfuzz._internal.system.environment.is_running_on_app_engine', ]) self.mock.is_running_on_app_engine.return_value = True + self.mock.can_user_access_testcase.return_value = True import server self.app = webtest.TestApp(server.app) @@ -58,3 +60,17 @@ def test_no_issue_url(self): response = self.app.get('/issue/12345', expect_errors=True) self.assertEqual(404, response.status_int) + + def test_no_access(self): + """A caller without access to the testcase must not be redirected to its + issue URL, previously this handler had no access check at all.""" + self.mock.can_user_access_testcase.return_value = False + testcase = data_types.Testcase() + testcase.bug_information = '456789' + self.mock.get_testcase.return_value = testcase + self.mock.get_issue_url.return_value = 'http://google.com/456789' + + response = self.app.get('/issue/12345', expect_errors=True) + + self.assertEqual(403, response.status_int) + self.mock.get_issue_url.assert_not_called()