Skip to content

Commit

Permalink
security: fix CVE-2024-37905 (#10230)
Browse files Browse the repository at this point in the history
Co-authored-by: Marc 'risson' Schmitt <marc.schmitt@risson.space>
  • Loading branch information
BeryJu and rissson authored Jun 26, 2024
1 parent 49fe670 commit a0c80ae
Show file tree
Hide file tree
Showing 4 changed files with 55 additions and 3 deletions.
7 changes: 7 additions & 0 deletions authentik/core/api/tokens.py
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,13 @@ def __init__(self, *args, **kwargs) -> None:
if SERIALIZER_CONTEXT_BLUEPRINT in self.context:
self.fields["key"] = CharField(required=False)

def validate_user(self, user: User):
"""Ensure user of token cannot be changed"""
if self.instance and self.instance.user_id:
if user.pk != self.instance.user_id:
raise ValidationError("User cannot be changed")
return user

Check warning on line 52 in authentik/core/api/tokens.py

View check run for this annotation

Codecov / codecov/patch

authentik/core/api/tokens.py#L49-L52

Added lines #L49 - L52 were not covered by tests

def validate(self, attrs: dict[Any, str]) -> dict[Any, str]:
"""Ensure only API or App password tokens are created."""
request: Request = self.context.get("request")
Expand Down
23 changes: 20 additions & 3 deletions authentik/core/tests/test_token_api.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,8 @@
USER_ATTRIBUTE_TOKEN_MAXIMUM_LIFETIME,
Token,
TokenIntents,
User,
)
from authentik.core.tests.utils import create_test_admin_user
from authentik.core.tests.utils import create_test_admin_user, create_test_user

Check warning on line 17 in authentik/core/tests/test_token_api.py

View check run for this annotation

Codecov / codecov/patch

authentik/core/tests/test_token_api.py#L17

Added line #L17 was not covered by tests
from authentik.lib.generators import generate_id


Expand All @@ -24,7 +23,7 @@ class TestTokenAPI(APITestCase):

def setUp(self) -> None:
super().setUp()
self.user = User.objects.create(username="testuser")
self.user = create_test_user()

Check warning on line 26 in authentik/core/tests/test_token_api.py

View check run for this annotation

Codecov / codecov/patch

authentik/core/tests/test_token_api.py#L26

Added line #L26 was not covered by tests
self.admin = create_test_admin_user()
self.client.force_login(self.user)

Expand Down Expand Up @@ -154,6 +153,24 @@ def test_token_create_expiring_custom_api(self):
self.assertEqual(token.expiring, True)
self.assertNotEqual(token.expires.timestamp(), expires.timestamp())

def test_token_change_user(self):

Check warning on line 156 in authentik/core/tests/test_token_api.py

View check run for this annotation

Codecov / codecov/patch

authentik/core/tests/test_token_api.py#L156

Added line #L156 was not covered by tests
"""Test creating a token and then changing the user"""
ident = generate_id()
response = self.client.post(reverse("authentik_api:token-list"), {"identifier": ident})
self.assertEqual(response.status_code, 201)
token = Token.objects.get(identifier=ident)
self.assertEqual(token.user, self.user)
self.assertEqual(token.intent, TokenIntents.INTENT_API)
self.assertEqual(token.expiring, True)
self.assertTrue(self.user.has_perm("authentik_core.view_token_key", token))
response = self.client.put(

Check warning on line 166 in authentik/core/tests/test_token_api.py

View check run for this annotation

Codecov / codecov/patch

authentik/core/tests/test_token_api.py#L158-L166

Added lines #L158 - L166 were not covered by tests
reverse("authentik_api:token-detail", kwargs={"identifier": ident}),
data={"identifier": "user_token_poc_v3", "intent": "api", "user": self.admin.pk},
)
self.assertEqual(response.status_code, 400)
token.refresh_from_db()
self.assertEqual(token.user, self.user)

Check warning on line 172 in authentik/core/tests/test_token_api.py

View check run for this annotation

Codecov / codecov/patch

authentik/core/tests/test_token_api.py#L170-L172

Added lines #L170 - L172 were not covered by tests

def test_list(self):
"""Test Token List (Test normal authentication)"""
Token.objects.all().delete()
Expand Down
27 changes: 27 additions & 0 deletions website/docs/security/CVE-2024-37905.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
# CVE-2024-37905

_Reported by [@m2a2](https://github.com/m2a2)_

## Improper Authorization for Token modification

### Summary

Due to insufficient permission checks it was possible for any authenticated user to elevate their permissions to a superuser by creating an API token and changing the user the token belonged to.

### Patches

authentik 2024.6.0, 2024.4.3 and 2024.2.4 fix this issue, for other versions the workaround can be used.

### Details

By setting a token's user ID to the ID of a higher privileged user, the token will inherit the higher privileged access to the API. This can be used to change the password of the affected user or to modify the authentik configuration in a potentially malicious way.

### Workarounds

As a workaround it is possible to block any requests to `/api/v3/core/tokens*` at the reverse-proxy/load-balancer level. Doing so prevents this issue from being exploited.

### For more information

If you have any questions or comments about this advisory:

- Email us at [security@goauthentik.io](mailto:security@goauthentik.io)
1 change: 1 addition & 0 deletions website/sidebars.js
Original file line number Diff line number Diff line change
Expand Up @@ -511,6 +511,7 @@ const docsSidebar = {
"security/security-hardening",
"security/policy",
"security/CVE-2024-38371",
"security/CVE-2024-37905",
"security/CVE-2024-23647",
"security/CVE-2024-21637",
"security/CVE-2023-48228",
Expand Down

0 comments on commit a0c80ae

Please sign in to comment.