From 972dce1462736992304111ac92956d628cf4b23e Mon Sep 17 00:00:00 2001 From: Jens L Date: Thu, 2 Mar 2023 20:15:33 +0100 Subject: [PATCH] security: fix CVE-2023-26481 (#4832) fix CVE-2023-26481 Signed-off-by: Jens Langhammer --- authentik/flows/views/executor.py | 2 +- authentik/stages/email/stage.py | 12 ++--- authentik/stages/email/tests/test_stage.py | 45 +++++++++++++++++-- .../flows-recovery-email-verification.yaml | 1 + website/docs/policies/expression.mdx | 2 +- website/docs/security/CVE-2023-26481.md | 27 +++++++++++ website/sidebars.js | 3 +- 7 files changed, 81 insertions(+), 11 deletions(-) create mode 100644 website/docs/security/CVE-2023-26481.md diff --git a/authentik/flows/views/executor.py b/authentik/flows/views/executor.py index 1505b0ae1..ae7968c7e 100644 --- a/authentik/flows/views/executor.py +++ b/authentik/flows/views/executor.py @@ -162,7 +162,7 @@ class FlowExecutorView(APIView): token.delete() if not isinstance(plan, FlowPlan): return None - plan.context[PLAN_CONTEXT_IS_RESTORED] = True + plan.context[PLAN_CONTEXT_IS_RESTORED] = token self._logger.debug("f(exec): restored flow plan from token", plan=plan) return plan diff --git a/authentik/stages/email/stage.py b/authentik/stages/email/stage.py index 02f118676..f116f7de0 100644 --- a/authentik/stages/email/stage.py +++ b/authentik/stages/email/stage.py @@ -15,7 +15,7 @@ from authentik.flows.challenge import Challenge, ChallengeResponse, ChallengeTyp from authentik.flows.models import FlowToken from authentik.flows.planner import PLAN_CONTEXT_IS_RESTORED, PLAN_CONTEXT_PENDING_USER from authentik.flows.stage import ChallengeStageView -from authentik.flows.views.executor import QS_KEY_TOKEN, SESSION_KEY_GET +from authentik.flows.views.executor import QS_KEY_TOKEN from authentik.stages.email.models import EmailStage from authentik.stages.email.tasks import send_mails from authentik.stages.email.utils import TemplateEmailMessage @@ -103,12 +103,14 @@ class EmailStageView(ChallengeStageView): def get(self, request: HttpRequest, *args, **kwargs) -> HttpResponse: # Check if the user came back from the email link to verify - if QS_KEY_TOKEN in request.session.get( - SESSION_KEY_GET, {} - ) and self.executor.plan.context.get(PLAN_CONTEXT_IS_RESTORED, False): + restore_token: FlowToken = self.executor.plan.context.get(PLAN_CONTEXT_IS_RESTORED, None) + user = self.get_pending_user() + if restore_token: + if restore_token.user != user: + self.logger.warning("Flow token for non-matching user, denying request") + return self.executor.stage_invalid() messages.success(request, _("Successfully verified Email.")) if self.executor.current_stage.activate_user_on_success: - user = self.get_pending_user() user.is_active = True user.save() return self.executor.stage_ok() diff --git a/authentik/stages/email/tests/test_stage.py b/authentik/stages/email/tests/test_stage.py index 45023a9cb..32141ba45 100644 --- a/authentik/stages/email/tests/test_stage.py +++ b/authentik/stages/email/tests/test_stage.py @@ -7,10 +7,9 @@ from django.core.mail.backends.smtp import EmailBackend as SMTPEmailBackend from django.urls import reverse from django.utils.http import urlencode -from authentik.core.models import Token from authentik.core.tests.utils import create_test_admin_user, create_test_flow from authentik.flows.markers import StageMarker -from authentik.flows.models import FlowDesignation, FlowStageBinding +from authentik.flows.models import FlowDesignation, FlowStageBinding, FlowToken from authentik.flows.planner import PLAN_CONTEXT_PENDING_USER, FlowPlan from authentik.flows.tests import FlowTestCase from authentik.flows.views.executor import QS_KEY_TOKEN, SESSION_KEY_PLAN @@ -134,7 +133,7 @@ class TestEmailStage(FlowTestCase): session = self.client.session session[SESSION_KEY_PLAN] = plan session.save() - token: Token = Token.objects.get(user=self.user) + token: FlowToken = FlowToken.objects.get(user=self.user) with patch("authentik.flows.views.executor.FlowExecutorView.cancel", MagicMock()): # Call the executor shell to preseed the session @@ -165,3 +164,43 @@ class TestEmailStage(FlowTestCase): plan: FlowPlan = session[SESSION_KEY_PLAN] self.assertEqual(plan.context[PLAN_CONTEXT_PENDING_USER], self.user) self.assertTrue(plan.context[PLAN_CONTEXT_PENDING_USER].is_active) + + def test_token_invalid_user(self): + """Test with token with invalid user""" + # Make sure token exists + self.test_pending_user() + self.user.is_active = False + self.user.save() + plan = FlowPlan(flow_pk=self.flow.pk.hex, bindings=[self.binding], markers=[StageMarker()]) + session = self.client.session + session[SESSION_KEY_PLAN] = plan + session.save() + # Set flow token user to a different user + token: FlowToken = FlowToken.objects.get(user=self.user) + token.user = create_test_admin_user() + token.save() + + with patch("authentik.flows.views.executor.FlowExecutorView.cancel", MagicMock()): + # Call the executor shell to preseed the session + url = reverse( + "authentik_api:flow-executor", + kwargs={"flow_slug": self.flow.slug}, + ) + url_query = urlencode( + { + QS_KEY_TOKEN: token.key, + } + ) + url += f"?query={url_query}" + self.client.get(url) + + # Call the actual executor to get the JSON Response + response = self.client.get( + reverse( + "authentik_api:flow-executor", + kwargs={"flow_slug": self.flow.slug}, + ) + ) + + self.assertEqual(response.status_code, 200) + self.assertStageResponse(response, component="ak-stage-access-denied") diff --git a/blueprints/example/flows-recovery-email-verification.yaml b/blueprints/example/flows-recovery-email-verification.yaml index 1f4c6ca43..7c143d172 100644 --- a/blueprints/example/flows-recovery-email-verification.yaml +++ b/blueprints/example/flows-recovery-email-verification.yaml @@ -154,6 +154,7 @@ entries: policy: !KeyOf default-recovery-skip-if-restored target: !KeyOf flow-binding-email order: 0 + state: absent model: authentik_policies.policybinding attrs: negate: false diff --git a/website/docs/policies/expression.mdx b/website/docs/policies/expression.mdx index 729f6e637..b6d9545bf 100644 --- a/website/docs/policies/expression.mdx +++ b/website/docs/policies/expression.mdx @@ -99,7 +99,7 @@ This includes the following: - `context['application']`: The application the user is in the process of authorizing. (Optional) - `context['source']`: The source the user is authenticating/enrolling with. (Optional) - `context['pending_user']`: The currently pending user, see [User](../user-group/user.md#object-attributes) -- `context['is_restored']`: Set to `True` when the flow plan has been restored from a flow token, for example the user clicked a link to a flow which was sent by an email stage. (Optional) +- `context['is_restored']`: Contains the flow token when the flow plan was restored from a link, for example the user clicked a link to a flow which was sent by an email stage. (Optional) - `context['auth_method']`: Authentication method (this value is set by password stages) (Optional) Depending on method, `context['auth_method_args']` is also set. diff --git a/website/docs/security/CVE-2023-26481.md b/website/docs/security/CVE-2023-26481.md new file mode 100644 index 000000000..3a6106a1d --- /dev/null +++ b/website/docs/security/CVE-2023-26481.md @@ -0,0 +1,27 @@ +# CVE-2023-26481 + +_Reported by [@fuomag9](https://github.com/fuomag9)_ + +## Insufficient user check in FlowTokens by Email stage + +### Summary + +Due to an insufficient access check, a recovery flow link that is created by an admin (or sent via email by an admin) can be used to set the password for any arbitrary user. + +### Patches + +authentik 2022.12.3, 2023.1.3, 2023.2.3 fix this issue. + +### Impact + +This attack is only possible if a recovery flow exists, which has both an Identification and an Email stage bound to it. If the flow has policies on the identification stage to skip it when the flow is restored (by checking `request.context['is_restored']`), the flow is not affected by this. With this flow in place, an administrator must create a recovery Link or send a recovery URL to the attacker, who can, due to the improper validation of the token create, set the password for any account. + +### Workaround + +It is recommended to upgrade to the patched version of authentik. Regardless, for custom recovery flows it is recommended to add a policy that checks if the flow is restored, and skips the identification stage. + +### For more information + +If you have any questions or comments about this advisory: + +- Email us at [security@goauthentik.io](mailto:security@goauthentik.io) diff --git a/website/sidebars.js b/website/sidebars.js index 6ad07b3cb..e77f03393 100644 --- a/website/sidebars.js +++ b/website/sidebars.js @@ -300,9 +300,10 @@ module.exports = { }, items: [ "security/policy", + "security/CVE-2022-23555", "security/CVE-2022-46145", "security/CVE-2022-46172", - "security/CVE-2022-23555", + "security/CVE-2023-26481", ], }, ],