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
Prevent XSS attack using /auth/enable_cookies #1455
Prevent XSS attack using /auth/enable_cookies #1455
Conversation
| @@ -40,7 +40,7 @@ export default function createEnableCookies({ | |||
|
|
|||
| <script> | |||
| window.apiKey = "${apiKey}"; | |||
| window.shopOrigin = "https://${shop}"; | |||
| window.shopOrigin = "https://${encodeURIComponent(shop)}"; | |||
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.
Should the https:// be part of the encodeURIComponent call as well?
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.
i'm pretty sure "shop" always comes back as foo.myshopify.com
a couple years ago, @ragalie did this
3284be1#diff-a15b7c0928331371cc40785dd637c644R16
which is a different way of fixing this, in a different place
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.
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.
That makes sense
| @@ -39,7 +39,7 @@ export default function createRequestStorageAccess({ | |||
|
|
|||
| <script> | |||
| window.apiKey = "${apiKey}"; | |||
| window.shopOrigin = "https://${shop}"; | |||
| window.shopOrigin = "https://${encodeURIComponent(shop)}"; | |||
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.
Same thing here.
|
In general, I have a feeling that this PR is wrong. The |
Good point and I agree with you! |
|
FYI @nhusher @seamusabshere we have a bug bounty program over at https://hackerone.com/shopify I am thinking of two rules in particular:
|
https://example.com/shopify/auth/enable_cookies?shop=%27%22%3C/title%3E%3C/style%3E%3C/textarea%3E%3C/noscript%3E%3C/script%3E--%3E%3Cdtfy%3E%3Cscript%3Ealert(%27hello%20world%27)%3C/script%3E
Thanks to @nhusher
Description
Fixes #1453