CVE-2019-16766: Insufficient Information (NVD-CWE-noinfo)
value: When using wagtail-2fa before 1.3.0, if someone gains access to someone's Wagtail login credentials, they can log into the CMS and bypass the 2FA check by changing the URL. They can then add a new device and gain full access to the CMS. This problem has been patched in version 1.3.0.
CWE description: There is insufficient information about the issue to classify it; details are unkown or unspecified.
CVSS v3 base score: 8.8
Published: 2019-11-29T17:15Z
Repository: https://github.com/labd/wagtail-2fa
Language: Python
Files changed: src/wagtail_2fa/middleware.py, tests/test_middleware.py
Commit date: 2019-11-26 12:36:37 +0100
Commit message: Allow a limited list of paths when user has device

This prevents users from adding or listing devices while they are not
yet verified and already have a registered device.

Diff: diff --git a/src/wagtail_2fa/middleware.py b/src/wagtail_2fa/middleware.py
index a3a7504..74d73f6 100644
--- a/src/wagtail_2fa/middleware.py
+++ b/src/wagtail_2fa/middleware.py
@@ -8,10 +8,15 @@ from django.utils.functional import SimpleLazyObject
 from django_otp.middleware import OTPMiddleware as _OTPMiddleware
 
 
 class VerifyUserMiddleware(_OTPMiddleware):
     _allowed_url_names = [
+        "wagtail_2fa_auth",
+        "wagtailadmin_login",
+        "wagtailadmin_logout",
+    ]
+    _allowed_url_names_no_device = [
         "wagtail_2fa_auth",
         "wagtail_2fa_device_list",
         "wagtail_2fa_device_new",
         "wagtail_2fa_device_qrcode",
         "wagtailadmin_login",
@@ -64,28 +69,30 @@ class VerifyUserMiddleware(_OTPMiddleware):
             or user.has_perms(["wagtailadmin.access_admin"])
         ):
             return False
 
         # Allow the user to a fixed number of paths when not verified
-        if request.path in self._allowed_paths:
+        user_has_device = django_otp.user_has_device(user, confirmed=True)
+        if request.path in self._get_allowed_paths(user_has_device):
             return False
 
         # For all other cases require that the user is verfied via otp
         return True
 
-    @property
-    def _allowed_paths(self):
-        """Return the paths the user may visit when not verified via otp
-
-        This result cannot be cached since we want to be compatible with the
-        django-hosts package. Django-hosts alters the urlconf based on the
-        hostname in the request, so the urls might exist for admin.<domain> but
-        not for www.<domain>.
+    def _get_allowed_paths(self, has_device):
+        """Return the paths the user may visit when not verified via otp.
 
+        If the user already has a registered device, return a limited set of
+        paths to prevent them from adding or listing devices to prevent them
+        from adding or listing devices.
         """
+        allowed_url_names = self._allowed_url_names
+        if not has_device:
+            allowed_url_names = self._allowed_url_names_no_device
+
         results = []
-        for route_name in self._allowed_url_names:
+        for route_name in allowed_url_names:
             try:
                 results.append(settings.WAGTAIL_MOUNT_PATH + reverse(route_name))
             except NoReverseMatch:
                 pass
         return results
diff --git a/tests/test_middleware.py b/tests/test_middleware.py
index 0fc5568..4db9d86 100644
--- a/tests/test_middleware.py
+++ b/tests/test_middleware.py
@@ -48,17 +48,17 @@ def test_superuser_dont_require_register_device(rf, superuser, settings):
     assert response is None
 
 
 def test_not_specifiying_wagtail_mount_point_does_not_prepend_allowed_paths_with_wagtail_mount_path(settings):
     settings.WAGTAIL_MOUNT_PATH = ''
-    allowed_paths = VerifyUserMiddleware()._allowed_paths
+    allowed_paths = VerifyUserMiddleware()._get_allowed_paths(has_device=False)
 
     for allowed_path in allowed_paths:
         assert allowed_path.startswith('/cms')
 
 
 def test_specifiying_wagtail_mount_point_does_prepend_allowed_paths_with_wagtail_mount_path(settings):
     settings.WAGTAIL_MOUNT_PATH = '/wagtail'
-    allowed_paths = VerifyUserMiddleware()._allowed_paths
+    allowed_paths = VerifyUserMiddleware()._get_allowed_paths(has_device=False)
 
     for allowed_path in allowed_paths:
         assert allowed_path.startswith(settings.WAGTAIL_MOUNT_PATH)

Vulnerable code:         if request.path in self._allowed_paths:
    @property
    def _allowed_paths(self):
        """Return the paths the user may visit when not verified via otp

        This result cannot be cached since we want to be compatible with the
        django-hosts package. Django-hosts alters the urlconf based on the
        hostname in the request, so the urls might exist for admin.<domain> but
        not for www.<domain>.
        for route_name in self._allowed_url_names:
    allowed_paths = VerifyUserMiddleware()._allowed_paths
    allowed_paths = VerifyUserMiddleware()._allowed_paths
Fixed code:         "wagtail_2fa_auth",
        "wagtailadmin_login",
        "wagtailadmin_logout",
    ]
    _allowed_url_names_no_device = [
        user_has_device = django_otp.user_has_device(user, confirmed=True)
        if request.path in self._get_allowed_paths(user_has_device):
    def _get_allowed_paths(self, has_device):
        """Return the paths the user may visit when not verified via otp.
        If the user already has a registered device, return a limited set of
        paths to prevent them from adding or listing devices to prevent them
        from adding or listing devices.
        allowed_url_names = self._allowed_url_names
        if not has_device:
            allowed_url_names = self._allowed_url_names_no_device

        for route_name in allowed_url_names:
    allowed_paths = VerifyUserMiddleware()._get_allowed_paths(has_device=False)
    allowed_paths = VerifyUserMiddleware()._get_allowed_paths(has_device=False)