Security: protect against open redirects

[afeld]

The origin set in session.py is used for history and (as of #414) redirection after login.

benefits/benefits/core/session.py

Lines 237 to 239 in c8689c6

	if origin is not None:
	logger.debug(f"Update session {_ORIGIN}")
	request.session[_ORIGIN] = origin

Currently, all cases where the origin are set are "trusted", but I could imagine a case where we take that path from a query parameter, leaving ourselves vulnerable to an open redirect vulnerability. We should validate the URL/path being passed.

Info about open redirects:

• https://affinity-it-security.com/what-is-an-open-redirect/
• https://dzone.com/articles/what-is-an-open-redirection-vulnerability-and-how
• https://cheatsheetseries.owasp.org/cheatsheets/Unvalidated_Redirects_and_Forwards_Cheat_Sheet.html

[thekaveman]

A straightforward way to implement this protection might be to move the usage of Django's reverse() helper from the callers of update() into the function itself.

That would embed and enforce the assumption that the origin is a safe identifier for a known Django application route. A call to reverse() with an invalid route name or a random external URL would fail, causing the update to fail.

E.g. where right now, say in a view, we might call update() like:

session.update(request, origin=reverse("eligibility:start"))

We change the session.update() code as follows:

if origin is not None: 
     logger.debug(f"Update session {_ORIGIN}") 
     request.session[_ORIGIN] = reverse(origin)

And callers like:

session.update(request, origin="eligibility:start")

Though we have a few places where a direct URL is passed that would need to be updated, e.g. in the agency_index() view function. In that example we'd need to pass an extra parameter into the update() method to be able to resolve the agency's index page with the core:agency_index route definition.

[afeld]

How Django does it:

https://github.com/django/django/blob/13a9cde133ac82e33dd091ca9bb9c677804afbe1/django/contrib/auth/views.py#L74-L84

[afeld]

Looked through the app and we don't have any redirects coming from parameters, so not going to worry about this for now.