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

Extensions protected urls 4928 #2779

Closed
wants to merge 6 commits into from

Conversation

fmarier
Copy link
Member

@fmarier fmarier commented Jun 25, 2019

Draft PR to fix brave/brave-browser#4928.

@bridiver Do you think an override is the best way to patch this?

Submitter Checklist:

Test Plan:

The webRequest hack can manually tested using https://github.com/fmarier/webrequest-demo and the contentScript one using https://github.com/fmarier/contentscript-demo.

Reviewer Checklist:

  • New files have MPL-2.0 license header.
  • Request a security/privacy review as needed.
  • Adequate test coverage exists to prevent regressions
  • Verify test plan is specified in PR before merging to source

After-merge Checklist:

  • The associated issue milestone is set to the smallest version that the
    changes has landed on.
  • All relevant documentation has been updated.

@fmarier fmarier requested a review from bridiver June 25, 2019 01:40
@fmarier fmarier self-assigned this Jun 25, 2019
if (IsRestrictedUrl(document_url, error))
return PageAccess::kDenied;

+ if (IsBraveRestrictedUrl(document_url, extension_id_, location_, error)) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

per dm this patch can be removed by using an ExtensionRegistryObserver to add the withheld patterns in either OnExtensionLoaded or OnExtensionReady. At that point we can check extensions::PermissionsData::CanExecuteScriptEverywhere for the extension and decide whether or not to add it and no patch will be necessary

@fmarier fmarier force-pushed the extensions-protected-urls-4928 branch from e8b04d2 to a28c1ff Compare July 6, 2019 01:56
@fmarier fmarier force-pushed the extensions-protected-urls-4928 branch from a28c1ff to a4e19bd Compare July 10, 2019 00:59
@fmarier fmarier force-pushed the extensions-protected-urls-4928 branch from a4e19bd to 802d463 Compare July 13, 2019 03:43
@fmarier fmarier force-pushed the extensions-protected-urls-4928 branch from 802d463 to 4105f86 Compare July 17, 2019 20:27
@fmarier
Copy link
Member Author

fmarier commented Aug 15, 2019

Superseded by #2946

@fmarier fmarier closed this Aug 15, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

For Uphold wallet integration, we should protect against malicious extensions
2 participants