False positive: py/url-redirection does not recognise sanitisation by checking netloc

[max-schaefer]

Description of the false positive

A common suggestion for checking whether a URL is safe to redirect to (found, for example, on StackOverflow) is to parse it and check that the netloc is empty, meaning that the URL doesn't specify a host name. The CodeQL query does not recognise this as a sanitiser.

In general, I think the following is a reasonable sanitiser:

untrusted = untrusted.replace("\\", "/")
if urlparse(untrusted).netloc == '':
  # fine to redirect, `untrusted` is a relative path, not a URL

(Note that the bit about replacing backslashes with slashes is important since urlparse doesn't treat backslashes the way many browses do.)

Code samples or links to source code

For example, the following snippet is flagged, though I think it is safe:

from urllib.parse import urlparse

@app.route('/example')
def example():
    untrusted = request.args.get('target', '')
    untrusted = untrusted.replace("\\", "/")
    if urlparse(untrusted).netloc == '':
        return redirect(untrusted, code=302)
    return redirect("/", code=302)

[max-schaefer]

I have a work-in-progress branch that partly addresses this: https://github.com/github/codeql/tree/max-schaefer/py-url-redirection

The main shortcoming at the moment is that it doesn't track whether backslashes have been eliminated, so as it stands the query on that branch would let insufficient sanitisation slip through, which is perhaps not what we want.

[aeisenberg]

Thanks for reporting and picking this up!