Security: Validate issue ID against testcase in update-issue handler#5307
Open
Ashutosh0x wants to merge 1 commit into
Open
Security: Validate issue ID against testcase in update-issue handler#5307Ashutosh0x wants to merge 1 commit into
Ashutosh0x wants to merge 1 commit into
Conversation
The /testcase-detail/update-issue endpoint authorizes access to the testcase but then performs issue-tracker writes using a request-supplied issueId without verifying it matches the testcase's currently linked issue. This allows a user with access to one testcase to trigger updates (comments, labels, title changes, rebinding) on an unrelated issue. Add a check that the supplied issueId matches the testcase's existing bug_information field before proceeding with the update. If the testcase has no linked issue yet (first-time linking), the check is skipped to preserve existing functionality. Fixes google#5262
Author
|
Hi @jonathanmetzman — this fixes a cross-object authorization bypass in the /testcase-detail/update-issue handler (issue #5262). The handler checks access on the testcase but accepts issueId from the request body without validating it matches the testcase's linked issue. A user with access to testcase A (linked to issue 1111) can supply issueId=2222 to write comments or modify labels on a completely unrelated issue. On OSS-Fuzz this could enable cross-project privilege escalation. The fix adds a simple validation — if the supplied issueId doesn't match the testcase's existing �ug_information, the request is rejected with a 403. Minimal change, no breaking behavior for legitimate usage. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fix cross-object authorization issue in
/testcase-detail/update-issuewhere a user with access to one testcase can trigger issue-tracker writes on an unrelated issue by supplying a differentissueIdin the request.Problem
The handler at
update_issue.pyperforms access control checks on the testcase (via@handler.check_testcase_access), but theissueIdparameter is accepted directly from the request without any validation against the testcase's currently linked issue.This means:
issueIdis taken from request input (attacker-controlled)A user authorized for testcase A (linked to issue 1111) can supply
issueId=2002to update issue 2002: adding comments, modifying labels/title, and rebinding the testcase.Fix
Add a check that verifies the request-supplied
issueIdmatches the testcase's existingbug_informationbefore performing any issue-tracker operations. If the testcase has no linked issue yet (first-time linking), the check is skipped to preserve existing functionality.issue_id = helpers.cast(issue_id, int, 'Issue ID (%s) is not a number!' % issue_id) + + # Verify that the supplied issue ID matches the testcase's currently linked + # issue. Without this check, a user authorized for one testcase could use + # that authorization to trigger issue-tracker writes on an arbitrary issue. + existing_issue_id = testcase.bug_information + if existing_issue_id and str(existing_issue_id) != str(issue_id): + raise helpers.EarlyExitError( + 'The supplied issue ID (%d) does not match the issue currently ' + 'linked to this testcase (%s). You cannot update an unrelated ' + 'issue through this endpoint.' % (issue_id, existing_issue_id), 403)Impact
Without this fix, a user with testcase-scoped access can cause the trusted issue-tracker integration to:
Fixes #5262