From c0c11c551cc48e48c70abea259b7b6e696c980ed Mon Sep 17 00:00:00 2001 From: Nicolas Date: Sat, 13 Jun 2026 17:55:35 +0200 Subject: [PATCH] fix: enforce single-use TOTP passcodes across all 2FA surfaces The web 2FA login and password-reset paths validated the passcode and then wrote LastUsedPasscode in a non-atomic read-check-write sequence, so two parallel submissions of the same code could each authenticate (TOCTOU). The Basic-Auth X-Gitea-OTP path never recorded the used passcode at all, letting a captured code be replayed for its whole validity window. Add TwoFactor.ValidateAndConsumeTOTP, which validates and atomically marks the passcode used via a conditional UPDATE (rejecting replays and racing duplicates), and route the web login, password-reset, and Basic-Auth paths through it. Assisted-by: Claude:claude-opus-4-8 --- models/auth/twofactor.go | 23 ++++++++++++++ models/auth/twofactor_test.go | 47 +++++++++++++++++++++++++++++ routers/web/auth/2fa.go | 12 ++------ routers/web/auth/password.go | 10 ++---- services/auth/basic.go | 3 +- tests/integration/api_twofa_test.go | 6 ++++ 6 files changed, 83 insertions(+), 18 deletions(-) create mode 100644 models/auth/twofactor_test.go diff --git a/models/auth/twofactor.go b/models/auth/twofactor.go index 9c0208f47f5..894b55f5d48 100644 --- a/models/auth/twofactor.go +++ b/models/auth/twofactor.go @@ -21,6 +21,7 @@ import ( "github.com/pquerna/otp/totp" "golang.org/x/crypto/pbkdf2" + "xorm.io/builder" ) // @@ -118,6 +119,28 @@ func (t *TwoFactor) ValidateTOTP(passcode string) (bool, error) { return totp.Validate(passcode, secretStr), nil } +// ValidateAndConsumeTOTP validates the passcode and atomically records it as used so that the +// same passcode cannot be redeemed more than once (RFC 6238 ยง5.2). It returns false for an +// invalid passcode as well as for a replay, including the case where a concurrent request with +// the same passcode won the race first. All TOTP login surfaces must go through this helper. +func (t *TwoFactor) ValidateAndConsumeTOTP(ctx context.Context, passcode string) (bool, error) { + ok, err := t.ValidateTOTP(passcode) + if err != nil || !ok { + return false, err + } + // Conditional update: only a row whose stored passcode differs from this one is updated, so a + // replay (or a concurrent duplicate) matches zero rows and is rejected. The row lock taken by + // the UPDATE serializes racing requests, closing the read-validate-write TOCTOU window. + t.LastUsedPasscode = passcode + n, err := db.GetEngine(ctx).ID(t.ID). + Where(builder.Or(builder.IsNull{"last_used_passcode"}, builder.Neq{"last_used_passcode": passcode})). + Cols("last_used_passcode").Update(t) + if err != nil { + return false, err + } + return n == 1, nil +} + // NewTwoFactor creates a new two-factor authentication token. func NewTwoFactor(ctx context.Context, t *TwoFactor) error { _, err := db.GetEngine(ctx).Insert(t) diff --git a/models/auth/twofactor_test.go b/models/auth/twofactor_test.go new file mode 100644 index 00000000000..1da5814e03f --- /dev/null +++ b/models/auth/twofactor_test.go @@ -0,0 +1,47 @@ +// Copyright 2026 The Gitea Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package auth_test + +import ( + "testing" + "time" + + auth_model "gitea.dev/models/auth" + "gitea.dev/models/unittest" + + "github.com/pquerna/otp/totp" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestTwoFactorValidateAndConsumeTOTP(t *testing.T) { + require.NoError(t, unittest.PrepareTestDatabase()) + + key, err := totp.Generate(totp.GenerateOpts{SecretSize: 40, Issuer: "gitea-test", AccountName: "consume"}) + require.NoError(t, err) + + tfa := &auth_model.TwoFactor{UID: 1} + require.NoError(t, tfa.SetSecret(key.Secret())) + require.NoError(t, auth_model.NewTwoFactor(t.Context(), tfa)) + + passcode, err := totp.GenerateCode(key.Secret(), time.Now()) + require.NoError(t, err) + + // first use of a valid passcode succeeds + ok, err := tfa.ValidateAndConsumeTOTP(t.Context(), passcode) + require.NoError(t, err) + assert.True(t, ok) + + // replaying the same passcode is refused, even when still inside the TOTP validity window + reloaded, err := auth_model.GetTwoFactorByUID(t.Context(), tfa.UID) + require.NoError(t, err) + ok, err = reloaded.ValidateAndConsumeTOTP(t.Context(), passcode) + require.NoError(t, err) + assert.False(t, ok) + + // an invalid passcode is rejected without consuming anything + ok, err = reloaded.ValidateAndConsumeTOTP(t.Context(), "000000") + require.NoError(t, err) + assert.False(t, ok) +} diff --git a/routers/web/auth/2fa.go b/routers/web/auth/2fa.go index 77d90c4da14..10376842a3c 100644 --- a/routers/web/auth/2fa.go +++ b/routers/web/auth/2fa.go @@ -58,14 +58,14 @@ func TwoFactorPost(ctx *context.Context) { return } - // Validate the passcode with the stored TOTP secret. - ok, err := twofa.ValidateTOTP(form.Passcode) + // Validate the passcode and atomically consume it to prevent reuse/replay. + ok, err := twofa.ValidateAndConsumeTOTP(ctx, form.Passcode) if err != nil { ctx.ServerError("UserSignIn", err) return } - if ok && twofa.LastUsedPasscode != form.Passcode { + if ok { remember := ctx.Session.Get("twofaRemember").(bool) u, err := user_model.GetUserByID(ctx, id) if err != nil { @@ -81,12 +81,6 @@ func TwoFactorPost(ctx *context.Context) { } } - twofa.LastUsedPasscode = form.Passcode - if err = auth.UpdateTwoFactor(ctx, twofa); err != nil { - ctx.ServerError("UserSignIn", err) - return - } - _ = ctx.Session.Set(session.KeyUserHasTwoFactorAuth, true) handleSignIn(ctx, u, remember) return diff --git a/routers/web/auth/password.go b/routers/web/auth/password.go index a650f243ec7..880a4e2d803 100644 --- a/routers/web/auth/password.go +++ b/routers/web/auth/password.go @@ -177,23 +177,17 @@ func ResetPasswdPost(ctx *context.Context) { regenerateScratchToken = true } else { passcode := ctx.FormString("passcode") - ok, err := twofa.ValidateTOTP(passcode) + ok, err := twofa.ValidateAndConsumeTOTP(ctx, passcode) if err != nil { ctx.HTTPError(http.StatusInternalServerError, "ValidateTOTP", err.Error()) return } - if !ok || twofa.LastUsedPasscode == passcode { + if !ok { ctx.Data["IsResetForm"] = true ctx.Data["Err_Passcode"] = true ctx.RenderWithErrDeprecated(ctx.Tr("auth.twofa_passcode_incorrect"), tplResetPassword, nil) return } - - twofa.LastUsedPasscode = passcode - if err = auth.UpdateTwoFactor(ctx, twofa); err != nil { - ctx.ServerError("ResetPasswdPost: UpdateTwoFactor", err) - return - } } } diff --git a/services/auth/basic.go b/services/auth/basic.go index c7db14e6e7e..909b026919b 100644 --- a/services/auth/basic.go +++ b/services/auth/basic.go @@ -176,7 +176,8 @@ func validateTOTP(req *http.Request, u *user_model.User) error { } return err } - if ok, err := twofa.ValidateTOTP(req.Header.Get("X-Gitea-OTP")); err != nil { + // Consume the passcode atomically so a captured OTP cannot be replayed within its validity window. + if ok, err := twofa.ValidateAndConsumeTOTP(req.Context(), req.Header.Get("X-Gitea-OTP")); err != nil { return err } else if !ok { return util.NewInvalidArgumentErrorf("invalid provided OTP") diff --git a/tests/integration/api_twofa_test.go b/tests/integration/api_twofa_test.go index d8d8fa44971..141b7e4cdcf 100644 --- a/tests/integration/api_twofa_test.go +++ b/tests/integration/api_twofa_test.go @@ -51,6 +51,12 @@ func TestAPITwoFactor(t *testing.T) { AddBasicAuth(user.Name) req.Header.Set("X-Gitea-OTP", passcode) MakeRequest(t, req, http.StatusOK) + + // the same passcode must not be replayable on the basic-auth surface (RFC 6238 single-use) + req = NewRequest(t, "GET", "/api/v1/user"). + AddBasicAuth(user.Name) + req.Header.Set("X-Gitea-OTP", passcode) + MakeRequest(t, req, http.StatusUnauthorized) } func TestBasicAuthWithWebAuthn(t *testing.T) {