scalableminds / webknossos

Visualize, share and annotate your large 3D images online
https://webknossos.org
GNU Affero General Public License v3.0
124 stars 23 forks source link

Enhance webknossos security #1685

Closed daniel-wer closed 6 years ago

daniel-wer commented 7 years ago

In my recent talk I've pointed out several issues and ways to enhance webknossos security, which should be addressed.

Log Time

jfrohnhofen commented 7 years ago

Probably related: On prod master, the dataset description now contain html as text, that is no longer evaluated: bildschirmfoto 2017-02-14 um 21 02 35

daniel-wer commented 7 years ago

That's not a bug, that's a feature :D Of course we can allow html in the dataset description if it's needed, I wasn't aware of someone using it and couldn't find a dataset on master that has an html description either (the one in the screenshot no longer has one).

normanrz commented 7 years ago

Can we allow "safe" HTML there? Perhaps Markdown?

fm3 commented 6 years ago

@daniel-wer are the remaining items here still open? Is any of that easily done? Or can we possibly close this issue?

daniel-wer commented 6 years ago

(1)

Adjust Same-Origin Policy (CORS headers) for Datastore to no longer be "*"

As we're using a GET token to authenticate requests to the datastore (not a Cookie which would be sent automatically by the browser), the SOP is not that important. If a user visits a malicious website which makes a request to the datastore, the request will be unauthenticated and not return anything.

(2)

Include X-Frame-Options: DENY Cookie to prevent Click-Jacking

I would do that, otherwise webKnossos will be open to click jacking attacks. I've prepared a quick demo for that, if you're logged in to webknossos in another tab, a malicious website could do this: https://jsfiddle.net/t6werxs7/

(3)

Investigate and determine a reasonable Content-Security-Policy to prevent possible future XSS issues

As we're using react to render almost everything, which takes care of escaping user supplied data, this does not have high priority. We're not using dangerouslySetInnerHTML, user supplied href prop values or HTML enabled markdown - all things that could lead to XSS even when using React - so I would argue we're pretty safe. It would nevertheless be worthwhile to investigate this as part of another issue and proactively create a Content-Security-Policy (https://content-security-policy.com/) to avoid future possible XSS vectors.

Summarizing, (1) is not needed, (2) should be easily done, (3) worthwhile but low-pri, I'll create a separate issue for that.

fm3 commented 6 years ago

Thanks for investigating! I’ll look into (2) then, and close this issue when it’s done. For reference: https://www.playframework.com/documentation/2.4.x/SecurityHeaders

daniel-wer commented 6 years ago

Great, I created issue #3085 to track progress regarding the CSP.