Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Upgrade to Django 4.2 #275

Closed
ERosendo opened this issue Jun 22, 2023 · 3 comments
Closed

Upgrade to Django 4.2 #275

ERosendo opened this issue Jun 22, 2023 · 3 comments

Comments

@ERosendo
Copy link
Contributor

While working on #258, we discovered that the url_has_allowed_host_and_scheme method does not use the urllib.parse module. Django copied the code to parse URLs( urlsplit, urlparse) from the urllib module, but version 4.1 does not include the logic to fix the vulnerability related to ASCII newlines and tabs in URLs(python/cpython#88048), even though it was already fixed in Python 3.11, so we decided to use the same approach we use in CL in the is_safe_url method.

OTOH, Django 4.2 includes a helper to fix the vulnerability, so we can refactor the is_safe_url method and remove the logic to prevent using garbage URLs (like empty ones or ones with spaces) and dangerous URLs (like JavaScript). The refactored function should look like this:

def is_safe_url(url: str, request: HttpRequest) -> bool:
    sign_in_url = reverse("sign-in") in url
    register_in_url = reverse("register") in url
    not_safe_url = not url_has_allowed_host_and_scheme(
        url,
        allowed_hosts={request.get_host()},
        require_https=request.is_secure(),
    )
    if any([sign_in_url, register_in_url, not_safe_url]):
        return False
    return True
@mlissner mlissner changed the title Refactor is_safe_url method after upgrading Django to version 4.2 Upgrade to Django 4.2 Jun 24, 2023
@mlissner
Copy link
Member

I just updated the title of the issue so we can put all our 4.2 upgrade notes here. I'm putting it on your backlog as well. I think it should be pretty quick.

@mlissner mlissner moved this to 🤖Bots Backlog in @erosendo's backlog Jun 25, 2023
@mlissner
Copy link
Member

This is done, right? Want to take a look at CL too and see if it needs changes?

@ERosendo
Copy link
Contributor Author

Yes. This is done.

Sure! I'll take a look.

@github-project-automation github-project-automation bot moved this from Main Backlog to Done in @erosendo's backlog Jul 10, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

No branches or pull requests

2 participants