clowder-framework / clowder

A data management system that allows users to share, annotate, organize and analyze large collections of datasets. It provides support for extensible metadata annotation using JSON-LD and a distribute analytics event bus for automatic curation of uploaded data.
https://clowderframework.org/
University of Illinois/NCSA Open Source License
33 stars 17 forks source link

password reset link security vulnerability #447

Open tcnichol opened 2 months ago

tcnichol commented 2 months ago

Host Header Injection: By capture the POST request for a password reset sent to https://cpmr.tacc.utexas.edu/reset, a malicious user can edit the Host entry in the header. This will modify the password reset link sent to the user to direct them to the attacker's site.

tcnichol commented 2 months ago

This may fix the issue. Need to upgrade secure social. https://github.com/jaliss/securesocial/issues/601

longshuicy commented 2 months ago

@lmarini and I brainstormed some possible fix:

  1. instead of directly using "@securesocial.core.providers.utils.RoutesHelper.signUp(token).absoluteURL(IdentityProvider.sslEnabled)" compare this link with the host name to see if they match 2. use the "host_ip" environment variable or introduce another hostname environment variable
  2. when compare, if doesn't match, replace the hostname with that

In below places: https://github.com/clowder-framework/clowder/blob/f28c203c56b2d4690d32ea0bce5364458de1ec79/app/views/ss/mails/passwordResetEmail.scala.html#L8

and

https://github.com/clowder-framework/clowder/blob/f28c203c56b2d4690d32ea0bce5364458de1ec79/app/views/ss/mails/signUpEmail.scala.html#L8

longshuicy commented 2 months ago

Host_IP see here: https://github.com/clowder-framework/clowder/blob/f28c203c56b2d4690d32ea0bce5364458de1ec79/conf/application.conf#L24

In the .html template, here is an example of how you get variable (e.g. sortInMemory)

https://github.com/clowder-framework/clowder/blob/f28c203c56b2d4690d32ea0bce5364458de1ec79/app/views/spaces/collectionsGrid.scala.html#L7

play.Play.application().configuration().getBoolean("sortInMemory")
robkooper commented 2 months ago

Problem is that the hostIp is almost never set, this was a hack for one of the previewers. Since the URL can change we tried to not hardcode this in the configuration.

The following code try to see what the URL should be.

https://github.com/clowder-framework/clowder/blob/develop/app/util/RequestUtils.scala#L15

https://github.com/clowder-framework/clowder/blob/develop/app/controllers/Utils.scala#L18

longshuicy commented 2 months ago

Problem is that the hostIp is almost never set, this was a hack for one of the previewers. Since the URL can change we tried to not hardcode this in the configuration.

The following code try to see what the URL should be.

https://github.com/clowder-framework/clowder/blob/develop/app/util/RequestUtils.scala#L15

https://github.com/clowder-framework/clowder/blob/develop/app/controllers/Utils.scala#L18

Maybe it's time we introduce another config variables? The idea is we need to have control on the host in the email sent out to avoid host header injection.. i think...

longshuicy commented 2 months ago

Problem is that the hostIp is almost never set, this was a hack for one of the previewers. Since the URL can change we tried to not hardcode this in the configuration.

The following code try to see what the URL should be.

https://github.com/clowder-framework/clowder/blob/develop/app/util/RequestUtils.scala#L15

https://github.com/clowder-framework/clowder/blob/develop/app/controllers/Utils.scala#L18

I think the similar idea of getBaseUrlAndProtocol triggered this bug report in the first place? That's a known bug in secure social: https://github.com/jaliss/securesocial/issues/601