-
Notifications
You must be signed in to change notification settings - Fork 4.3k
Update Browser_Extension_Vulnerabilities_Cheat_Sheet.md #1923
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
base: master
Are you sure you want to change the base?
Conversation
szh
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not familiar with this API but I did some quick searching and I found that Google has additional guidelines on this: https://developer.chrome.com/docs/extensions/develop/concepts/messaging#content-scripts-are-less-trustworthy
Perhaps we should include their recommendations and link to this document?
|
Thank you for the pointing this out. I've changed the mitigation section, changed to be more action item(ish) list that contains all of the info shared in chrome document. @szh could you please review again? |
| ```javascript | ||
| chrome.runtime.onMessage.addListener((request, sender, sendResponse) => { | ||
| if (sender.id !== chrome.runtime.id) return; | ||
| if (!sender.url?.startsWith('chrome-extension://')) return; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Once we're checking if it comes from an extension, why not also check the extension ID, like if (!sender.url?.startsWith('chrome-extension://<my-extension-id>'))
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sender.id already uniquely identifies the extension; checking the ID again in the URL would be redundant. URL validation can still be used optionally to restrict specific extension contexts.
|
|
||
| ```javascript | ||
| chrome.runtime.onMessage.addListener((request, sender, sendResponse) => { | ||
| if (sender.id !== chrome.runtime.id) return; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should this id be the extension's id?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes. sender.id is the ID of the extension that sent the message.
Comparing it to chrome.runtime.id ensures the message originated from this same extension.
szh
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the explanations. LGTM
I've added chrome.runtime.sendMessage/onMessage check as a additional section.
This was missing on the CS
Please make sure that for your contribution:
[TEXT](URL)If your PR is related to an issue, please finish your PR text with the following line:
This PR fixes issue #
<REPLACE WITH ISSUE NUMBER>.AI Tool Usage Disclosure (required for all PRs)
Please select one of the following options:
the contents and I affirm the results. The LLM used is
[llm name and version]and the prompt used is
[your prompt here]. [Feel free to add more details if needed]Thank you again for your contribution 😃