-
Notifications
You must be signed in to change notification settings - Fork 0
π‘οΈ Sentinel: [HIGH] Fix insecure postMessage target origin and origin verification #18
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
97e544b
2fb9d3a
8679613
a96ffee
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,9 @@ | ||
| ## 2025-05-14 - [Insecure postMessage Target Origin in OAuth Redirect] | ||
| **Vulnerability:** The OAuth `/redirect` endpoint used `window.postMessage` with a wildcard (`'*'`) target origin, allowing any origin to intercept the authorization code. | ||
| **Learning:** In a multi-platform environment like Activepieces, the correct target origin must be resolved dynamically using the platform context to ensure the code is only sent to the trusted frontend. | ||
| **Prevention:** Always use `platformUtils.getPlatformIdForRequest(request)` and `domainHelper.getPublicUrl({ platformId })` to resolve the specific origin for `postMessage`. | ||
|
|
||
| ## 2025-05-14 - [Improper Origin Verification in Frontend OAuth Utility] | ||
| **Vulnerability:** The frontend `getCode` function used `startsWith` to verify the origin of `postMessage` events, which could be bypassed (e.g., `https://trusted.com.malicious.com` starts with `https://trusted.com`). | ||
| **Learning:** `startsWith` is insufficient for origin verification. | ||
| **Prevention:** Always use strict equality (`===`) for origin comparison and extract the origin component correctly using `new URL(url).origin`. |
| 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 publicUrl = await domainHelper.getPublicUrl({ platformId }) | ||
| const targetOrigin = new URL(publicUrl).origin | ||
| 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 successfully, this window should close now</html>`, | ||
|
Comment on lines
+256
to
+262
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.
The Useful? React with πΒ / π. |
||
| ) | ||
| } | ||
| }, | ||
|
|
||
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: 384
π Script executed:
Repository: AGI-Corporation/Route.X
Length of output: 889
π Script executed:
Repository: AGI-Corporation/Route.X
Length of output: 415
Bind OAuth message acceptance to the popup window (
event.source) as well as origin.event.origin === expectedOriginis necessary, but not sufficient for popup integrity. A different same-origin window can still post a forged{ code }message and satisfy this check.π Proposed hardening
function getCode(redirectUrl: string): Promise<string> { const expectedOrigin = new URL(redirectUrl).origin; return new Promise<string>((resolve) => { window.addEventListener('message', function handler(event) { if ( - redirectUrl && + event.source === currentPopup && event.origin === expectedOrigin && - event.data['code'] + typeof event.data === 'object' && + event.data !== null && + 'code' in event.data && + typeof (event.data as { code?: unknown }).code === 'string' ) { - resolve(decodeURIComponent(event.data.code)); + resolve(decodeURIComponent((event.data as { code: string }).code)); currentPopup?.close(); window.removeEventListener('message', handler); } }); }); }π€ Prompt for AI Agents