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

Enhance webknossos security #1685

Closed
4 of 5 tasks
daniel-wer opened this issue Feb 5, 2017 · 7 comments · Fixed by #1686 or #3084
Closed
4 of 5 tasks

Enhance webknossos security #1685

daniel-wer opened this issue Feb 5, 2017 · 7 comments · Fixed by #1686 or #3084

Comments

@daniel-wer
Copy link
Member

daniel-wer commented Feb 5, 2017

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

  • Fix popover XSS in task overview
  • Fix possible XSS caused by <%= that is being used in Backbone templates
  • Set security.javascriptEnabled to False in the Mongo config to prevent possible NoSql injections
  • [ ] Restrict lifetime of Play Session IDs (https://www.playframework.com/documentation/2.5.x/ScalaSessionFlash) → we don’t store anything in the sessions, cookie/auth token lifetime is separate
  • [ ] Adjust Same-Origin Policy (CORS headers) for Datastore to no longer be "*" → we use token-based authentication for datastore, no cookies
  • Include X-Frame-Options: DENY Cookie to prevent Click-Jacking
  • Investigate and determine a reasonable Content-Security-Policy to prevent possible future XSS issues

Log Time

@daniel-wer daniel-wer added this to the 17_03 Multi-lab deployment management milestone Feb 5, 2017
daniel-wer added a commit that referenced this issue Feb 5, 2017
@daniel-wer daniel-wer mentioned this issue Feb 5, 2017
1 task
daniel-wer added a commit that referenced this issue Feb 7, 2017
@tmbo tmbo reopened this Feb 7, 2017
@jfrohnhofen
Copy link
Contributor

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
Copy link
Member Author

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
Copy link
Member

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

@tmbo tmbo unassigned tmbo and daniel-wer Jun 7, 2017
@hotzenklotz hotzenklotz removed this from the 17_03 Multi-lab deployment management milestone Aug 22, 2017
@fm3 fm3 removed the backend-easy label Jun 13, 2018
@fm3
Copy link
Member

fm3 commented Aug 14, 2018

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

@daniel-wer daniel-wer added this to the Sprint 26b milestone Aug 20, 2018
@daniel-wer daniel-wer self-assigned this Aug 21, 2018
@daniel-wer
Copy link
Member Author

(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
Copy link
Member

fm3 commented Aug 21, 2018

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
Copy link
Member Author

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

@fm3 fm3 closed this as completed in #3084 Aug 22, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment