Prevent XSS attack using /auth/enable_cookies #1455
Conversation
ghost
added
the
| @@ -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.
Should the https:// be part of the encodeURIComponent call as well?
There was a problem hiding this comment.
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.
🤔 I think it's okay since we only want to encode the value coming from the user
| @@ -39,7 +39,7 @@ export default function createRequestStorageAccess({ | |||
|
|
|||
| <script> | |||
| window.apiKey = "${apiKey}"; | |||
| window.shopOrigin = "https://${shop}"; | |||
| window.shopOrigin = "https://${encodeURIComponent(shop)}"; | |||
|
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 💰 and there are rules for participation! 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