Require testcase access before disclosing crash_query duplicates#5390
Open
herdiyana256 wants to merge 1 commit into
Open
Require testcase access before disclosing crash_query duplicates#5390herdiyana256 wants to merge 1 commit into
herdiyana256 wants to merge 1 commit into
Conversation
crash_query's post handler passes project/crash_type/crash_state straight into find_testcase, which filters by those fields alone with no ownership check. Any authenticated user could pass an arbitrary project name and, if a matching open testcase existed there, get back its duplicate_id and bug_id. The handler already called access.can_user_access_testcase before disclosing a match, but only when the testcase was security-flagged. Non-security-flagged testcases in any other project were disclosed unconditionally. Apply the same access check regardless of the security flag, so a match is only disclosed to a caller who actually has access to that testcase (job/fuzzer ACL, uploader, or issue-tracker ownership). Added a test for the non-security-flagged cross-project case; the existing test only covered the security-flagged one.
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
1 participant
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.
crash_query's post handler passes project/crash_type/crash_state straight into find_testcase, which filters by those fields alone with no ownership check. Any authenticated user could pass an arbitrary project name and, if a matching open testcase existed there, get back its duplicate_id and bug_id.
The handler already called access.can_user_access_testcase before disclosing a match, but only when the testcase was security-flagged. Non-security-flagged testcases in any other project were disclosed unconditionally, regardless of whether the caller had any relationship to that project.
Applies the same access check regardless of the security flag, so a match is only disclosed to a caller who actually has access to that testcase (job/fuzzer ACL, uploader, or issue-tracker ownership) -- the same standard already used everywhere else a testcase gets shown to a user.
Added a test for the non-security-flagged cross-project case; the existing test only covered the security-flagged one.