Skip to content

Commit 26d094f

Browse files
committed
Fix critical security issue with misusing the Django Signer API
After the code was refactored to use the official Signer class the salt was passed wrongly as secret key, replacing the SECRET_KEY set in Django settings file. This allows, with verification enabled, to guess the signature by a potential attacker very easily. In reset password case, the guessing by attacker can be mitigated somewhat by using RESET_PASSWORD_VERIFICATION_ONE_TIME_USE but it leads to a different signature schema. This bug affects versions 0.2.*, 0.3.*, 0.4.*
1 parent dd7a2a5 commit 26d094f

File tree

4 files changed

+70
-3
lines changed

4 files changed

+70
-3
lines changed

rest_registration/verification.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@ def __init__(self, data):
3131
data[self.TIMESTAMP_FIELD] = get_current_timestamp()
3232
self._data = data
3333
self._salt = self._calculate_salt(data)
34-
self._signer = Signer(self._salt)
34+
self._signer = Signer(salt=self._salt)
3535

3636
def _calculate_signature(self, data):
3737
if self.SIGNATURE_FIELD in data:

tests/api/test_register.py

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,27 @@ def test_no_password_ok(self):
6767
)
6868

6969

70+
@override_settings(REST_REGISTRATION=REST_REGISTRATION_WITH_VERIFICATION)
71+
class RegisterSignerTestCase(TestCase):
72+
73+
def test_signer_with_different_secret_keys(self):
74+
user = self.create_test_user(is_active=False)
75+
data_to_sign = {'user_id': user.pk}
76+
secrets = [
77+
'#0ka!t#6%28imjz+2t%l(()yu)tg93-1w%$du0*po)*@l+@+4h',
78+
'feb7tjud7m=91$^mrk8dq&nz(0^!6+1xk)%gum#oe%(n)8jic7',
79+
]
80+
signatures = []
81+
for secret in secrets:
82+
with override_settings(
83+
SECRET_KEY=secret):
84+
signer = RegisterSigner(data_to_sign)
85+
data = signer.get_signed_data()
86+
signatures.append(data[signer.SIGNATURE_FIELD])
87+
88+
assert signatures[0] != signatures[1]
89+
90+
7091
def build_custom_verification_url(signer):
7192
base_url = signer.get_base_url()
7293
signed_data = signer.get_signed_data()

tests/api/test_register_email.py

Lines changed: 26 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66
from rest_framework.test import force_authenticate
77

88
from rest_registration.api.views.register_email import RegisterEmailSigner
9-
from tests.utils import shallow_merge_dicts
9+
from tests.utils import TestCase, shallow_merge_dicts
1010

1111
from .base import APIViewTestCase
1212

@@ -18,6 +18,31 @@
1818
}
1919

2020

21+
@override_settings(REST_REGISTRATION=REST_REGISTRATION_WITH_EMAIL_VERIFICATION)
22+
class RegisterEmailSignerTestCase(TestCase):
23+
24+
def test_signer_with_different_secret_keys(self):
25+
email = 'testuser1@example.com'
26+
user = self.create_test_user(is_active=False)
27+
data_to_sign = {
28+
'user_id': user.pk,
29+
'email': email,
30+
}
31+
secrets = [
32+
'#0ka!t#6%28imjz+2t%l(()yu)tg93-1w%$du0*po)*@l+@+4h',
33+
'feb7tjud7m=91$^mrk8dq&nz(0^!6+1xk)%gum#oe%(n)8jic7',
34+
]
35+
signatures = []
36+
for secret in secrets:
37+
with override_settings(
38+
SECRET_KEY=secret):
39+
signer = RegisterEmailSigner(data_to_sign)
40+
data = signer.get_signed_data()
41+
signatures.append(data[signer.SIGNATURE_FIELD])
42+
43+
assert signatures[0] != signatures[1]
44+
45+
2146
class BaseRegisterEmailViewTestCase(APIViewTestCase):
2247

2348
def setUp(self):

tests/api/test_reset_password.py

Lines changed: 22 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66
from rest_framework import status
77

88
from rest_registration.api.views.reset_password import ResetPasswordSigner
9-
from tests.utils import shallow_merge_dicts
9+
from tests.utils import TestCase, shallow_merge_dicts
1010

1111
from .base import APIViewTestCase
1212

@@ -18,6 +18,27 @@
1818
}
1919

2020

21+
@override_settings(REST_REGISTRATION=REST_REGISTRATION_WITH_RESET_PASSWORD)
22+
class ResetPasswordSignerTestCase(TestCase):
23+
24+
def test_signer_with_different_secret_keys(self):
25+
user = self.create_test_user(is_active=False)
26+
data_to_sign = {'user_id': user.pk}
27+
secrets = [
28+
'#0ka!t#6%28imjz+2t%l(()yu)tg93-1w%$du0*po)*@l+@+4h',
29+
'feb7tjud7m=91$^mrk8dq&nz(0^!6+1xk)%gum#oe%(n)8jic7',
30+
]
31+
signatures = []
32+
for secret in secrets:
33+
with override_settings(
34+
SECRET_KEY=secret):
35+
signer = ResetPasswordSigner(data_to_sign)
36+
data = signer.get_signed_data()
37+
signatures.append(data[signer.SIGNATURE_FIELD])
38+
39+
assert signatures[0] != signatures[1]
40+
41+
2142
@override_settings(
2243
REST_REGISTRATION=REST_REGISTRATION_WITH_RESET_PASSWORD,
2344
)

0 commit comments

Comments
 (0)