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

Removed textile #535

Merged
merged 2 commits into from
Jul 4, 2021
Merged

Removed textile #535

merged 2 commits into from
Jul 4, 2021

Conversation

vikiival
Copy link
Member

@vikiival vikiival commented Jul 4, 2021

Removing textile from project as we do not use it anymore.
@yangwao please also remove key from ENV on Netlify.

@vikiival vikiival added bug Something isn't working p2 core functionality, or is affecting 60% of app labels Jul 4, 2021
@vikiival vikiival self-assigned this Jul 4, 2021
@vikiival vikiival requested a review from yangwao July 4, 2021 10:18
@github-actions
Copy link
Contributor

github-actions bot commented Jul 4, 2021

You're Pull Request scored a 0.05 out of a possible +5 on the sentiment scale. Here's a gif representation of your PR:
Boo from Monsters Inc blinking and looking into space neutrally

@github-actions
Copy link
Contributor

github-actions bot commented Jul 4, 2021

Gif

@github-actions
Copy link
Contributor

github-actions bot commented Jul 4, 2021

Gif

@yangwao
Copy link
Member

yangwao commented Jul 4, 2021

closes #305 as we don't use Textile anyways for any critical operations.

@yangwao yangwao merged commit 7a33cfc into main Jul 4, 2021
@vikiival vikiival mentioned this pull request Jul 4, 2021
2 tasks
@yangwao
Copy link
Member

yangwao commented Jul 4, 2021

Removing textile from project as we do not use it anymore.
@yangwao please also remove key from ENV on Netlify.

Removed.

@x676f64
Copy link

x676f64 commented Jul 4, 2021

This issue is still unresolved as the Slate upload API key is exposed client-side and there are other issues with the application flow as files are upload prior to an on-chain transaction. To demonstrate these issues, I was able to upload a SVG graphic which contains a XSS payload without recording a transaction on-chain.

https://kodadot.mypinata.cloud/ipfs/bafkreif72qk6ykwj2dthg6ar6r3w5u5tsmv2rqig2epv3pkaoxz6wqhwaa

<?xml version="1.0" standalone="no"?>

  <!DOCTYPE svg PUBLIC "-//W3C//DTD SVG 1.1//EN" "http://www.w3.org/Graphics/SVG/1.1/DTD/svg11.dtd">
   <svg version="1.1" baseProfile="full" xmlns="http://www.w3.org/2000/svg">
 <rect width="300" height="100" style="fill:rgb(0,0,255);stroke-width:3;stroke:rgb(0,0,0)" />
   <script type="text/javascript">
   otob("This is not good");
   </script>
   </svg>

If this image is processed within your web application it will execute the embedded javascript payload. If this was weaponized, this exploit could potentially be used to manipulate a users transactions or wallet.

@x676f64
Copy link

x676f64 commented Jul 4, 2021

In case the example above isn't apparent as you need to load the image inside an HTML page context, I have also uploaded a javascript file containing the same xss payload which does execute in the context of your kodadot.mypinata.cloud endpoint. https://kodadot.mypinata.cloud/ipfs/bafkreidjoa34lblzi3kmh7ozn5ytuc5f7vuxslvw4qno5mvlv4qjdd7iwi

@yangwao
Copy link
Member

yangwao commented Jul 5, 2021

This issue is still unresolved as the Slate upload API key is exposed client-side and there are other issues with the application flow as files are upload prior to an on-chain transaction. To demonstrate these issues, I was able to upload a SVG graphic which contains a XSS payload without recording a transaction on-chain.

We are using Slate to put your files in the background to accelerate the pinning process till the user figures out filling up credentials, it's a sort of experimental way how to speed up the process. I guess till we'll introduce some authentication system, this will be always in place. We can think of running a script, which checks which files aren't minted with a particular IPFS hash and remove them if that is the case.

https://kodadot.mypinata.cloud/ipfs/bafkreif72qk6ykwj2dthg6ar6r3w5u5tsmv2rqig2epv3pkaoxz6wqhwaa

<?xml version="1.0" standalone="no"?>

  <!DOCTYPE svg PUBLIC "-//W3C//DTD SVG 1.1//EN" "http://www.w3.org/Graphics/SVG/1.1/DTD/svg11.dtd">
   <svg version="1.1" baseProfile="full" xmlns="http://www.w3.org/2000/svg">
 <rect width="300" height="100" style="fill:rgb(0,0,255);stroke-width:3;stroke:rgb(0,0,0)" />
   <script type="text/javascript">
   otob("This is not good");
   </script>
   </svg>

If this image is processed within your web application it will execute the embedded javascript payload. If this was weaponized, this exploit could potentially be used to manipulate a users transactions or wallet.

Speaking of XSS, I've noticed we forgot to set XSS headers. Adding

X-Frame-Options: DENY
X-XSS-Protection: 1; mode=block

should mitigate most of the basic stuff I guess, thanks for noticing!

I guess going through basic owasp stuff would be good for long-term security, revisit on scenarios on CSP, CSRF.. seems now we are getting D at https://securityheaders.com/?q=nft.kodadot.xyz&followRedirects=on
image

Speaking of *.pinata.cloud, we have no power there and we should reach out to pinata provider to add some headers there as seems terrible F, that's probably why your XSS works?
https://securityheaders.com/?q=https%3A%2F%2Fkodadot.mypinata.cloud%2Fipfs%2Fbafkreidjoa34lblzi3kmh7ozn5ytuc5f7vuxslvw4qno5mvlv4qjdd7iwi&followRedirects=on

@yangwao
Copy link
Member

yangwao commented Jul 5, 2021

Ok, seems got level up in few minutes. I'm happy to see if you can demonstrate that XSS now if you mint some SVG?
Use this version if so https://deploy-preview-539--nftkodadot.netlify.app/ till it will be merged to the production, not sure whenever you will read this

image

@yangwao
Copy link
Member

yangwao commented Jul 5, 2021

And we are at A-grade policy, let me look on permissions policy, seems something new https://scotthelme.co.uk/goodbye-feature-policy-and-hello-permissions-policy/
image

@yangwao
Copy link
Member

yangwao commented Jul 5, 2021

@x676f64 Hey, you should state it's PoC for minor components :)
Thanks to headers deployed to our production, you should not be able execute those XSS.
However, we will try to tackle those as well.
https://twitter.com/TaylorAnthony/status/1411787145056985088

@vikiival vikiival deleted the bye-textile branch July 8, 2021 11:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working p2 core functionality, or is affecting 60% of app
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants