]> git.ipfire.org Git - thirdparty/paperless-ngx.git/commitdiff
Fix: dont allow allauth redirects to any host (#5783)
authorshamoon <4887959+shamoon@users.noreply.github.com>
Fri, 16 Feb 2024 00:37:34 +0000 (16:37 -0800)
committerGitHub <noreply@github.com>
Fri, 16 Feb 2024 00:37:34 +0000 (16:37 -0800)
---------

Co-authored-by: Trenton H <797416+stumpylog@users.noreply.github.com>
src/paperless/adapter.py
src/paperless/tests/test_adapter.py
src/paperless/urls.py

index 98b0f11ba694083c1da8885ffb2965f68347521b..40f95bf301343b02acc688515f35672a5ed71da8 100644 (file)
@@ -1,4 +1,5 @@
 from allauth.account.adapter import DefaultAccountAdapter
+from allauth.core import context
 from allauth.socialaccount.adapter import DefaultSocialAccountAdapter
 from django.conf import settings
 from django.urls import reverse
@@ -10,6 +11,21 @@ class CustomAccountAdapter(DefaultAccountAdapter):
         # Override with setting, otherwise default to super.
         return getattr(settings, "ACCOUNT_ALLOW_SIGNUPS", allow_signups)
 
+    def is_safe_url(self, url):
+        # see https://github.com/paperless-ngx/paperless-ngx/issues/5780
+        from django.utils.http import url_has_allowed_host_and_scheme
+
+        # get_host already validates the given host, so no need to check it again
+        allowed_hosts = {context.request.get_host()} | set(settings.ALLOWED_HOSTS)
+
+        if "*" in allowed_hosts:
+            # dont allow wildcard to allow urls from any host
+            allowed_hosts.remove("*")
+            allowed_hosts.add(context.request.get_host())
+            return url_has_allowed_host_and_scheme(url, allowed_hosts=allowed_hosts)
+
+        return url_has_allowed_host_and_scheme(url, allowed_hosts=allowed_hosts)
+
 
 class CustomSocialAccountAdapter(DefaultSocialAccountAdapter):
     def is_open_for_signup(self, request, sociallogin):
index ca79cbce0277c9f71879f0359967d0699b32bcd3..f07e0b4225e547dfbdbea445b0b5ae11caecb8be 100644 (file)
@@ -1,7 +1,12 @@
+from unittest import mock
+
 from allauth.account.adapter import get_adapter
+from allauth.core import context
 from allauth.socialaccount.adapter import get_adapter as get_social_adapter
 from django.conf import settings
+from django.http import HttpRequest
 from django.test import TestCase
+from django.test import override_settings
 from django.urls import reverse
 
 
@@ -17,6 +22,31 @@ class TestCustomAccountAdapter(TestCase):
         settings.ACCOUNT_ALLOW_SIGNUPS = False
         self.assertFalse(adapter.is_open_for_signup(None))
 
+    def test_is_safe_url(self):
+        request = HttpRequest()
+        request.get_host = mock.Mock(return_value="example.com")
+        with context.request_context(request):
+            adapter = get_adapter()
+            with override_settings(ALLOWED_HOSTS=["*"]):
+
+                # True because request host is same
+                url = "https://example.com"
+                self.assertTrue(adapter.is_safe_url(url))
+
+            url = "https://evil.com"
+            # False despite wildcard because request host is different
+            self.assertFalse(adapter.is_safe_url(url))
+
+            settings.ALLOWED_HOSTS = ["example.com"]
+            url = "https://example.com"
+            # True because request host is same
+            self.assertTrue(adapter.is_safe_url(url))
+
+            settings.ALLOWED_HOSTS = ["*", "example.com"]
+            url = "//evil.com"
+            # False because request host is not in allowed hosts
+            self.assertFalse(adapter.is_safe_url(url))
+
 
 class TestCustomSocialAccountAdapter(TestCase):
     def test_is_open_for_signup(self):
index 0419b8e66ad4dae054a303fe4190b4a43fbfbd83..142f2792ded6bd16436b31447357ec8de1061367 100644 (file)
@@ -193,6 +193,7 @@ urlpatterns = [
         RedirectView.as_view(
             url=settings.STATIC_URL + "frontend/en-US/assets/%(path)s",
         ),
+        # TODO: with localization, this is even worse! :/
     ),
     # App logo
     re_path(
@@ -200,7 +201,6 @@ urlpatterns = [
         serve,
         kwargs={"document_root": os.path.join(settings.MEDIA_ROOT, "logo")},
     ),
-    # TODO: with localization, this is even worse! :/
     # login, logout
     path("accounts/", include("allauth.urls")),
     # Root of the Frontend