Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[hackerone] Request-OTR and debounce services use improper comparator for host cache #32230

Closed
pilgrim-brave opened this issue Aug 14, 2023 · 4 comments · Fixed by brave/brave-core#19675
Assignees
Labels
OS/Android Fixes related to Android browser functionality OS/Desktop QA/No release-notes/include security

Comments

@pilgrim-brave
Copy link

pilgrim-brave commented Aug 14, 2023

Despite both being canonicalized by GURL, the string hosts that end up in host_cache_ can still be not-quite-equal to the actual host. Need to either further canonicalize hosts before comparing, or use a comparator function like url::DomainIs.

This affects any service that maintains its own host cache, which currently includes Request-OTR and debounce services.

credit: https://hackerone.com/reports/2107964 by nishimunea

@pilgrim-brave pilgrim-brave self-assigned this Aug 14, 2023
@diracdeltas diracdeltas changed the title Request-OTR and debounce services use improper comparator for host cache [hackerone] Request-OTR and debounce services use improper comparator for host cache Aug 14, 2023
@pilgrim-brave pilgrim-brave added QA/Yes release-notes/include OS/Android Fixes related to Android browser functionality OS/Desktop labels Aug 14, 2023
@brave-builds brave-builds added this to the 1.59.x - Nightly milestone Aug 15, 2023
@LaurenWags
Copy link
Member

@pes10k @pilgrim-brave could we get a test plan for this and let us know if it needs to be checked on all OSes? If it doesn't need manual QA can change the label to QA/No.

@pes10k
Copy link
Contributor

pes10k commented Sep 15, 2023

labeled QA/No, thanks @LaurenWags !

@LaurenWags
Copy link
Member

sounds good, thanks @pes10k. Just confirming, is the release-notes/include label still accurate?

@pes10k
Copy link
Contributor

pes10k commented Sep 15, 2023

sure, though i think it'd be fine to just say

  • bug fixes in determining which origins and URLs triggered debouncing and request-OTR protections

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
OS/Android Fixes related to Android browser functionality OS/Desktop QA/No release-notes/include security
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants