Skip to content
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

fix: DATA-11516 Add targetOrigin for postMessage calls #43

Merged
merged 1 commit into from
Jan 10, 2024

Conversation

bc-rmalyavc
Copy link
Contributor

@bc-rmalyavc bc-rmalyavc commented Jan 9, 2024

What?

Per title

Why?

Currently we are passing a wildcard (*) as a target when calling the window.postMessage method. This allows attackers to intercept the messages. So that we need to specify target which should be current store's CP url.

Testing / Proof

Manually tested on prod store. Verified both dev and prod environments. Verified that the messages e.g. Cancel or Use this button click are only intercepted when the store URL is correct. F.e. on prod store when the app is running in dev mode, the buttons won't do anything.

@bigcommerce/team-data

Copy link

vercel bot commented Jan 9, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
ai-app-foundation ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jan 9, 2024 4:59pm

@bc-rmalyavc bc-rmalyavc marked this pull request as ready for review January 9, 2024 17:00
@bc-rmalyavc bc-rmalyavc requested a review from a team as a code owner January 9, 2024 17:00
@bc-rmalyavc bc-rmalyavc merged commit 61578d8 into main Jan 10, 2024
3 checks passed
@bc-rmalyavc bc-rmalyavc deleted the DATA-11516 branch January 10, 2024 11:40
@bc-prestonhuth
Copy link

This is a good PR to get feedback from @bigcommerce/frontend on since they've had to address this issue elsewhere before. I know @rtalvarez had to for instance back on Channels team. My main concern is whether we are considering all of the necessary scenarios in terms of what the CP URL could be.

PR description is very helpful btw @bc-rmalyavc. Thanks for all of the good info.

@bc-rmalyavc
Copy link
Contributor Author

Hey @bc-prestonhuth,
Thanks for your feedback.
As far as I know, this kind of apps can be only installed on production stores. So that there may be only one possible CP url. Otherwise we would need to cover URLs like my-integration.zone or .my-staging.zone. On the second thought there seemed to be no need to cover development environment either.
I'd appreciate any feedback from @bigcommerce/frontend, though in case I'm missing something here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants