Post-merge hardening: CSV LF, SECURITY.md, dependabot#16
Merged
Conversation
- csvSafe now also prefixes cells starting with \n (LF). Some spreadsheet parsers evaluate LF-prefixed cells; the prior fix only handled CR and tab. - SECURITY.md documents threat model, scope, and the known nonce-readable-from-page caveat in MAIN<->ISOLATED bridge (impact: forged finding display only, no exfil) - Dependabot configured for weekly GH Actions version bumps
Both popup.html and results.html had a hardcoded "v2.0" span that went stale at the v2.0 -> v2.1.0 bump. Read chrome.runtime.getManifest() at init time instead so it stays in sync.
There was a problem hiding this comment.
Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| - **Per-tab badge dot** is set on the first finding only; subsequent findings update the count but not the icon | ||
| - **5000 finding cap** with FIFO eviction. High-volume scans (heavy SPAs over a long session) will drop oldest findings | ||
| - **CSV export** prefixes a single-quote on cells starting with `=`, `+`, `-`, `@`, tab, or carriage return to neutralise Excel / Sheets formula injection. Line-feed prefix is not currently neutralised |
The globalNames scan ran at document_start when interceptor.js boots. At that point page scripts have not executed yet, so window.API_KEY / window.__NEXT_DATA__ / etc. are undefined and the entire pass was effectively dead code on real pages. Run scanGlobals() at document_start (cheap; catches anything already there), then again at DOMContentLoaded, then at load. A scannedGlobals Set dedupes so each name reports at most once.
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.
Post-merge follow-ups from auditing #15. Grew during work — scope expanded from the original 3 items to 5 commits.
Changes
js/results.js- CSV LF prefixcsvSafealready neutralised cells starting with=,+,-,@, tab, CR. Some spreadsheet parsers also evaluate LF-prefixed cells. Added\nto the prefix regex.SECURITY.mddata-kf-verifyattribute lives ondocumentElementbetweendocument_startanddocument_idle. Any page script that runs in that window can read it and forge__kf_finding__events. Impact is bounded to displaying a fake finding in the user's results view - no data exfil, no privileged API. Mitigation cost is high because Symbols don't cross JS realms andpostMessageis also page-visible..github/dependabot.ymlWeekly GH Actions version bumps, minor/patch grouped, max 5 open PRs.
Hardcoded
v2.0in popup + results pageBoth
popup.htmlandresults.htmlhad a hardcoded version span that went stale at the 2.0 -> 2.1.0 bump. Now reads fromchrome.runtime.getManifest()at init.Deferred window-global scan in
js/interceptor.jsThe
globalNamesscan ran atdocument_startwhen interceptor.js boots. At that point page scripts have not executed yet, sowindow.API_KEY/window.__NEXT_DATA__/ etc. are undefined and the entire pass was effectively dead code on real pages. Now runs atdocument_start,DOMContentLoaded, andload, with per-name dedupe via ascannedGlobalsSet.CHANGELOG.mdTracks 2.0.0 and 2.1.0 releases plus an unreleased section for the contents of this PR.
Verification
bash scripts/build.shbuilds both zipsweb-ext lint --source-dir dist/firefox --self-hostedclean (only the two pre-existingdata_collection_permissionsinformational warnings about strict_min_version=128)p/javascriptp/security-auditp/owasp-top-tenonjs/: zero findings