fix(security): validate offscreen sender origin and enforce audio chunk size limit#885
fix(security): validate offscreen sender origin and enforce audio chunk size limit#885Siddh2024 wants to merge 3 commits into
Conversation
Several icon-only buttons (toggle visibility, settings) lacked aria-label attributes, making them inaccessible to screen readers. Adds descriptive aria-label to: - Toggle key visibility button in popup - Settings button in popup header - Toggle visibility buttons in options page Fixes shouri123#865
github-actions
Bot
added
gssoc
🚀 Thank You for Contributing to Late-MeetPlease ensure that:
Thank you for contributing 💙 |
github-actions
Bot
added
bug
|
👋 Thank you @Siddh2024 for your contribution to Late-Meet!
Please review any automated suggestions or code review comments that may appear below! We will review your PR as soon as possible! Please consider starring the repository ⭐ to show your support! |
github-actions
Bot
added
type:code
📝 WalkthroughWalkthroughThe service worker now authorizes audio chunk senders and rejects payloads exceeding a 10 MB limit before queueing. The popup settings button adds a ChangesAudio chunk validation
Settings button tooltip
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related issues
Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/background.ts`:
- Around line 2031-2036: Update the sender validation in the message handler
around senderUrl to compare sender.url against the existing
OFFSCREEN_DOCUMENT_URL value, rather than accepting any URL ending with
"offscreen.html". Preserve the unauthorized response and early return for every
sender whose URL does not exactly match the extension’s offscreen document URL.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: ec340936-a391-4ed7-950f-c8de24885a48
📒 Files selected for processing (3)
src/background.tssrc/config.tssrc/popup.html
| const senderUrl = sender.url ?? ""; | ||
| if (senderUrl && !senderUrl.endsWith("offscreen.html")) { | ||
| console.warn("[LateMeet] chunk rejected — unauthorized sender:", senderUrl); | ||
| sendResponse({ success: false, error: "Unauthorized sender" }); | ||
| return; | ||
| } |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Strengthen sender origin validation.
Validating the sender by checking if the URL ends with "offscreen.html" is vulnerable to spoofing. A content script running on an external website with a matching path (e.g., https://evil-website.com/offscreen.html) would pass this check and could spam or overflow the audio queue.
Since OFFSCREEN_DOCUMENT_URL is already defined at line 44, you should use it to ensure the message strictly originates from your extension's offscreen document.
🔒️ Proposed fix to strictly validate the extension's offscreen URL
const senderUrl = sender.url ?? "";
- if (senderUrl && !senderUrl.endsWith("offscreen.html")) {
+ if (senderUrl && !senderUrl.startsWith(OFFSCREEN_DOCUMENT_URL)) {
console.warn("[LateMeet] chunk rejected — unauthorized sender:", senderUrl);
sendResponse({ success: false, error: "Unauthorized sender" });
return;
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const senderUrl = sender.url ?? ""; | |
| if (senderUrl && !senderUrl.endsWith("offscreen.html")) { | |
| console.warn("[LateMeet] chunk rejected — unauthorized sender:", senderUrl); | |
| sendResponse({ success: false, error: "Unauthorized sender" }); | |
| return; | |
| } | |
| const senderUrl = sender.url ?? ""; | |
| if (senderUrl && !senderUrl.startsWith(OFFSCREEN_DOCUMENT_URL)) { | |
| console.warn("[LateMeet] chunk rejected — unauthorized sender:", senderUrl); | |
| sendResponse({ success: false, error: "Unauthorized sender" }); | |
| return; | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/background.ts` around lines 2031 - 2036, Update the sender validation in
the message handler around senderUrl to compare sender.url against the existing
OFFSCREEN_DOCUMENT_URL value, rather than accepting any URL ending with
"offscreen.html". Preserve the unauthorized response and early return for every
sender whose URL does not exactly match the extension’s offscreen document URL.
1 participant
🚀 Pull Request: Origin Validation & Size Enforcement for Audio Chunks (#875)
Issue: Critical: Offscreen audio chunk messages lack origin validation — any extension page can inject fake audio data (#875)
Problem:
The OFFSCREEN_AUDIO_CHUNK message handler in src/background.ts accepted audio data from any extension page without verifying the sender was the legitimate offscreen document. This created a defense-in-depth gap where a compromised extension page could:
Additionally, there was no maximum size cap on the base64 payload, enabling multi-megabyte chunk attacks.
What Changed:
Origin Validation (src/background.ts):
Size Enforcement:
Defense Layers:
1 - URL origin check - background.ts
2 - Payload size cap - background.ts
3 - Session active check (existing) - background.ts
4 - Queue capacity check (existing) - background.ts
Testing:
Summary by CodeRabbit
Bug Fixes
Accessibility