Page_uri => Vulnerability to open redirect attacks

[philippfo9]

The attacker can provide a user with a link, which points to the trusted website. If the user opens the link, he will be immediately redirected to a website, which is controlled by the attacker.

This is due to the page_uri parameter that you can provide as state={"page_uri": "..."}
e.g.
http://localhost:3001/login/redirect#state={%22page_uri%22:%22https://www.google.com%22}

It would be good to restrict this parameter to only be able to point to approved domains. This could be achieved by a combination of whitelist filters as well as regular expressions.
It's not enough to simply check if it includes the domain e.g. https://www.example.com, because this can be circumvented by passing 'https://www.example.com@www.google.com/'

[MrSwitch]

Yes, that's correct it functions this way so in theory one could have authentication and navigation terminating on another domain. Not sure how useful that is but hey ho.

An attacker wont get anything from redirecting the user. Unless i'm missing something here.

[philippfo9]

He could get the authority of the domain and make the user believe, he is on the right site. If the user does believe that, the user could possibly enter sensible data there.

Is it possible to contribute to it and add some config in order to be able to specify some host, that you validate against in this case or in which ways is this page_uri used?

[MrSwitch]

He could get the authority of the domain and make the user believe, he is on the right site. If the user does believe that, the user could possibly enter sensible data there.

If you have the ability to push links to users then this problem exists... that's pretty much any social site. I dont believe this should be solved here. HelloJS is acting as a redirect for malformed links but isn't exposing anything which could be abused.

[mithodin]

I'd like to comment that this is very much an issue, if an attacker is able to craft links that use my domain, so that it looks legit even to someone checking the link. We are not going to be able to use hello.js any longer if this is not going to be fixed.

[MrSwitch]

@mithodin if you dont mind putting a constant on your redirect.html page, before including hello.js

Something like ...

// Scope any redirect URL's to the current domain... this would prevent `page_uri` being used to redirect to another domain
var HELLOJS_REDIRECT_URL = window.location.origin;

// Disable any redirect
var HELLOJS_REDIRECT_URL = false;

Hellojs could then simply check !Object.prototype.hasOwnProperty.call(window, 'HELLOJS_REDIRECT_URL') || page_uri.match(window.HELLOJS_REDIRECT_URL), before allowing any redirect to occur.

Thoughts?

[mithodin]

@MrSwitch could the same be achieved by passing an option to hello.init()? That would somehow make more sense to me than using a global variable. Otherwise, yes I think that would be an acceptable option.

[MrSwitch]

@mithodin well if you look at the default implementation of the redirect.html page view-source:https://adodson.com/hello.js/redirect.html it never actually calls hello.init(). In hindsight it's a bad design, perhaps in a future version this will be addressed.

[mithodin]

I see. Well, if this is the only way to get the info in there, yes. Makes sense to do it this way. But I also agree that the design maybe isn't the best.

[MrSwitch]

Please approve the PR #651 if it addresses your immediate security concerns.

[mithodin]

Are you sure I can do that? I don't see the option of leaving a review on the PR