From fdc82be17e1cab7125a76a537e193e1691898fa3 Mon Sep 17 00:00:00 2001 From: herdiyana256 Date: Fri, 21 Aug 2026 04:02:09 +0700 Subject: [PATCH] Require job access in revisions_info before disclosing component revisions The revisions_info handler is decorated with @handler.oauth, which authenticates the caller but does not authorize access to a specific job. It read the `job` request parameter and returned the component/repo/revision list for that job with no access check, so any authenticated caller could disclose the build composition (source components, repo URLs and revisions) of an arbitrary job, including jobs and projects they are not authorized for. Gate the lookup on access.has_access(job_type=job) and validate the job name, matching the other job-scoped handlers (coverage_report, fuzzer_stats). The core logic moves into get_component_revisions_list so the access-denied path is covered by a unit test. --- src/appengine/handlers/revisions_info.py | 69 ++++++++++++------ .../appengine/handlers/revisions_info_test.py | 71 +++++++++++++++++++ 2 files changed, 119 insertions(+), 21 deletions(-) create mode 100644 src/clusterfuzz/_internal/tests/appengine/handlers/revisions_info_test.py diff --git a/src/appengine/handlers/revisions_info.py b/src/appengine/handlers/revisions_info.py index dbb8255640f..6c53d0707fa 100644 --- a/src/appengine/handlers/revisions_info.py +++ b/src/appengine/handlers/revisions_info.py @@ -16,11 +16,57 @@ from flask import request from clusterfuzz._internal.build_management import revisions +from clusterfuzz._internal.datastore import data_types from handlers import base_handler +from libs import access from libs import handler from libs import helpers +def get_component_revisions_list(job_type, revision, revision_range): + """Resolve the component revision list for a job. + + The result is per-job data, so callers must be authorized for the job. + """ + if not job_type: + raise helpers.EarlyExitError('Job name cannot be empty.', 400) + + if not data_types.Job.VALID_NAME_REGEX.match(job_type): + raise helpers.EarlyExitError('Invalid job name.', 400) + + # Component revisions are per-job data; gate on job access like the other + # job-scoped handlers (e.g. coverage_report, fuzzer_stats). @handler.oauth + # authenticates the caller but does not authorize access to an arbitrary job, + # so without this any caller could disclose the component/repo/revision list + # for a job (and project) they cannot otherwise access. + if not access.has_access(job_type=job_type): + raise helpers.AccessDeniedError() + + if revision: + if not revision.isdigit(): + raise helpers.EarlyExitError('Revision is not an integer.', 400) + start_revision = end_revision = revision + elif revision_range: + try: + start_revision, end_revision = revision_range.split(':') + except: + raise helpers.EarlyExitError('Bad revision range.', 400) + + if not start_revision.isdigit(): + raise helpers.EarlyExitError('Start revision is not an integer.', 400) + if not end_revision.isdigit(): + raise helpers.EarlyExitError('End revision is not an integer.', 400) + else: + raise helpers.EarlyExitError('No revision specified.', 400) + + component_revisions_list = revisions.get_component_range_list( + start_revision, end_revision, job_type) + if not component_revisions_list: + raise helpers.EarlyExitError('Failed to get component revisions.', 400) + + return component_revisions_list + + class Handler(base_handler.Handler): """Information on a revision range.""" @@ -32,27 +78,8 @@ def get(self): revision = request.get('revision') revision_range = request.get('range') - if revision: - if not revision.isdigit(): - raise helpers.EarlyExitError('Revision is not an integer.', 400) - start_revision = end_revision = revision - elif revision_range: - try: - start_revision, end_revision = revision_range.split(':') - except: - raise helpers.EarlyExitError('Bad revision range.', 400) - - if not start_revision.isdigit(): - raise helpers.EarlyExitError('Start revision is not an integer.', 400) - if not end_revision.isdigit(): - raise helpers.EarlyExitError('End revision is not an integer.', 400) - else: - raise helpers.EarlyExitError('No revision specified.', 400) - - component_revisions_list = revisions.get_component_range_list( - start_revision, end_revision, job_type) - if not component_revisions_list: - raise helpers.EarlyExitError('Failed to get component revisions.', 400) + component_revisions_list = get_component_revisions_list( + job_type, revision, revision_range) return self.render( 'revisions-info.html', diff --git a/src/clusterfuzz/_internal/tests/appengine/handlers/revisions_info_test.py b/src/clusterfuzz/_internal/tests/appengine/handlers/revisions_info_test.py new file mode 100644 index 00000000000..74b2bd3c39d --- /dev/null +++ b/src/clusterfuzz/_internal/tests/appengine/handlers/revisions_info_test.py @@ -0,0 +1,71 @@ +# Copyright 2019 Google LLC +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +"""Tests for revisions_info handler.""" +import unittest + +from clusterfuzz._internal.tests.test_libs import helpers +from handlers import revisions_info +from libs import helpers as libs_helpers + + +class RevisionsInfoTest(unittest.TestCase): + """Tests for the revisions_info handler.""" + + def setUp(self): + helpers.patch_environ(self) + helpers.patch(self, [ + 'libs.access.has_access', + 'clusterfuzz._internal.build_management.revisions.' + 'get_component_range_list', + ]) + # Default to a caller that is allowed to access the job; the access-denied + # path is exercised explicitly in test_no_access. + self.mock.has_access.return_value = True + self.mock.get_component_range_list.return_value = [{ + 'component': 'src', + 'link_text': '1:2', + }] + + def test_has_access(self): + """Tests that an authorized caller gets the component revision list.""" + result = revisions_info.get_component_revisions_list('job1', '1', None) + self.assertEqual([{'component': 'src', 'link_text': '1:2'}], result) + self.mock.has_access.assert_called_with(job_type='job1') + + def test_no_access(self): + """Tests that a caller without access to the job is denied instead of + being handed the component/repo/revision list for that job.""" + self.mock.has_access.return_value = False + with self.assertRaises(libs_helpers.AccessDeniedError): + revisions_info.get_component_revisions_list('job1', '1', None) + self.mock.has_access.assert_called_with(job_type='job1') + self.assertFalse(self.mock.get_component_range_list.called) + + def test_invalid_job_name(self): + """Tests that an invalid job name is rejected before any access check or + data lookup.""" + with self.assertRaises(libs_helpers.EarlyExitError): + revisions_info.get_component_revisions_list('bad job name!', '1', None) + self.assertFalse(self.mock.has_access.called) + self.assertFalse(self.mock.get_component_range_list.called) + + def test_empty_job_name(self): + """Tests that an empty job name is rejected.""" + with self.assertRaises(libs_helpers.EarlyExitError): + revisions_info.get_component_revisions_list('', '1', None) + self.assertFalse(self.mock.has_access.called) + + +if __name__ == '__main__': + unittest.main()