-
Notifications
You must be signed in to change notification settings - Fork 0
π‘οΈ Sentinel: [HIGH] Fix insecure postMessage in OAuth redirect flow #16
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: main
Are you sure you want to change the base?
Changes from all commits
08da125
4069558
c18dd89
1cc585d
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,11 @@ | ||
| ## 2025-05-15 - [HIGH] Secure OAuth2 Redirect postMessage | ||
|
|
||
| **Vulnerability:** The application was using the wildcard `*` as the `targetOrigin` in `window.postMessage` during the OAuth2 redirect flow, and it was using `startsWith` for origin verification when receiving the message. | ||
|
|
||
| **Learning:** | ||
| 1. Using `*` as a target origin allows any malicious site that can get a reference to the window (e.g., via `window.opener` if the popup was opened from another site or if the site was compromised) to intercept the sensitive OAuth code. | ||
| 2. Using `startsWith` for origin validation is insecure because it can be bypassed. For example, `https://example.com.attacker.com` starts with `https://example.com`. | ||
|
|
||
| **Prevention:** | ||
| 1. Always specify a precise `targetOrigin` when calling `postMessage`. In the frontend, use `window.location.origin`. On the server, resolve the platform's public URL and extract the origin. | ||
| 2. Always use strict equality (`===`) when verifying `event.origin` in a message listener to ensure it exactly matches the expected trusted origin. |
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -74,14 +74,18 @@ function constructUrl(params: OAuth2PopupParams, pckeChallenge: string) { | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| function getCode(redirectUrl: string): Promise<string> { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return new Promise<string>((resolve) => { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| window.addEventListener('message', function handler(event) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if ( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| redirectUrl && | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| redirectUrl.startsWith(event.origin) && | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| event.data['code'] | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| resolve(decodeURIComponent(event.data.code)); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| currentPopup?.close(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| window.removeEventListener('message', handler); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (!redirectUrl) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| try { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const expectedOrigin = new URL(redirectUrl).origin; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (event.origin === expectedOrigin && event.data['code']) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| resolve(decodeURIComponent(event.data.code)); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| currentPopup?.close(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| window.removeEventListener('message', handler); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } catch (e) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| console.error('Error parsing redirectUrl origin', e); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
74
to
91
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Origin verification is now secure, but the promise can hang indefinitely. The strict equality check (
Consider adding a timeout or rejection mechanism to prevent the OAuth flow from hanging indefinitely when something goes wrong. π‘ Suggested improvement with timeout function getCode(redirectUrl: string): Promise<string> {
- return new Promise<string>((resolve) => {
- window.addEventListener('message', function handler(event) {
+ return new Promise<string>((resolve, reject) => {
+ const timeoutId = setTimeout(() => {
+ window.removeEventListener('message', handler);
+ reject(new Error('OAuth code retrieval timed out'));
+ }, 120000); // 2 minute timeout
+
+ function handler(event: MessageEvent) {
if (!redirectUrl) {
+ clearTimeout(timeoutId);
+ window.removeEventListener('message', handler);
+ reject(new Error('redirectUrl is required'));
return;
}
try {
const expectedOrigin = new URL(redirectUrl).origin;
if (event.origin === expectedOrigin && event.data['code']) {
+ clearTimeout(timeoutId);
resolve(decodeURIComponent(event.data.code));
currentPopup?.close();
window.removeEventListener('message', handler);
}
} catch (e) {
console.error('Error parsing redirectUrl origin', e);
+ clearTimeout(timeoutId);
+ window.removeEventListener('message', handler);
+ reject(e);
}
- });
+ }
+
+ window.addEventListener('message', handler);
});
}π Committable suggestion
Suggested change
π€ Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -82,6 +82,7 @@ import { pieceMetadataServiceHooks } from './pieces/piece-metadata-service/hooks | |
| import { pieceSyncService } from './pieces/piece-sync-service' | ||
| import { platformModule } from './platform/platform.module' | ||
| import { platformService } from './platform/platform.service' | ||
| import { platformUtils } from './platform/platform.utils' | ||
| import { projectHooks } from './project/project-hooks' | ||
| import { projectModule } from './project/project-module' | ||
| import { storeEntryModule } from './store-entry/store-entry.module' | ||
|
|
@@ -250,12 +251,15 @@ export const setupApp = async (app: FastifyInstance): Promise<FastifyInstance> = | |
| return reply.send('The code is missing in url') | ||
| } | ||
| else { | ||
| const platformId = await platformUtils.getPlatformIdForRequest(request) | ||
| const frontendUrl = await domainHelper.getPublicUrl({ platformId }) | ||
| const targetOrigin = new URL(frontendUrl).origin | ||
|
Comment on lines
+254
to
+256
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
In cloud setups that configure Useful? React with πΒ / π. |
||
| return reply | ||
| .type('text/html') | ||
| .send( | ||
| `<script>if(window.opener){window.opener.postMessage({ 'code': '${encodeURIComponent( | ||
| params.code, | ||
| )}' },'*')}</script> <html>Redirect succuesfully, this window should close now</html>`, | ||
| )}' }, '${targetOrigin}')}</script> <html>Redirect succuesfully, this window should close now</html>`, | ||
| ) | ||
| } | ||
| }, | ||
|
|
||
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.
π§© Analysis chain
π Script executed:
Repository: AGI-Corporation/Route.X
Length of output: 24928
π Script executed:
Repository: AGI-Corporation/Route.X
Length of output: 110
π Script executed:
Repository: AGI-Corporation/Route.X
Length of output: 114
π Script executed:
# Read the complete oauth2-utils.ts file to understand the full context cat -n packages/react-ui/src/lib/oauth2-utils.tsRepository: AGI-Corporation/Route.X
Length of output: 3447
π Script executed:
Repository: AGI-Corporation/Route.X
Length of output: 1219
Fix the
postMessagetarget origin inredirect.tsxto match the opener's origin.The listener in
oauth2-utils.tsexpects messages fromhttps://secrets.activepieces.com, but the actual problem is inredirect.tsx(line 12-17): it posts withtargetOrigin: window.location.origin(its own origin athttps://secrets.activepieces.com).When the popup posts a message to the opener on
tenant.activepieces.comwithtargetOrigin: 'https://secrets.activepieces.com', the browser blocks it because the opener is not on that domain. The popup should post to the opener's origin instead, either viawindow.opener.location.originor'*'for compatibility across all tenant domains.This will silently fail in CLOUD_OAUTH2 flowsβthe popup sends the message, but the browser discards it before the opener receives it.
π€ Prompt for AI Agents