Python: open redirect protection example is still vulnerable

[stsewd]

The open redirect protection for this example is still vulnerable

codeql/python/ql/src/Security/CWE-601/examples/redirect_good2.py

Lines 8 to 12 in dea9229

	target = request.args.get('target', '')
	target = target.replace('\\', '')
	if not urlparse(target).netloc:
	# relative path, safe to redirect
	return redirect(target, code=302)

A target like https:/example.com (notice the single /) will be parsed as having no netloc, but browsers will redirect to https://example.com (tested on Firefox and Chrome using Fedora).

from urllib.parse import urlparse

print(urlparse('https:/example.com'))
# ParseResult(scheme='https', netloc='', path='/example.com', params='', query='', fragment='')

See Django for example

https://github.com/django/django/blob/f339c4c8e4870f23d3ba8bf8ee68c57628739592/django/utils/http.py#L356-L361

[RasmusWL]

Thanks 👍 We recently changed this example, thanks for letting us know about this 💪 I'll look into rewriting our example to account for this edge-case, unless you have a suggestion for a "safe" rewrite?

(I'll also look into ensuring that our modeling recognize this edge-case)

[stsewd]

unless you have a suggestion for a "safe" rewrite?

Hmm, good question. I was about to suggest to check for the protocol as well like Django does, but in the past I've solved this by just forcing the target to start with one / (check for \ as well) or by adding the hostname to it.

For A:

• //example.com -> /example.com
• https://example.com -> /https://example.com

For B:

• //example.com -> https://mydomain.com//example.com
• https://example.com -> https://mydomain.com/https://example.com

[RasmusWL]

This one got away from me, but I've created #16646 now to address the issue 👍

[RasmusWL]

Even though the PR is merged, it will take a little time for the documentation page to show the new text+example (since that is only updated after we make a release that contains these changes)