There is a TOCTOU (Time-of-Check-Time-of-Use) race condition in the setup flow:
SetupRequired() middleware checks if any users exist
HandleSetupPOST() processes the form
CreateUser() checks UserExists() again, then inserts
If two requests arrive simultaneously (e.g., automated attack while the setup page is accessible), both can pass the UserExists() check before either has inserted a user. The second insert may succeed (SQLite doesn't have a UNIQUE constraint on username based on the migration), creating a second admin account.
Even if the second CreateUser returns ErrUserExists, the middleware check at step 1 happens independently for each request.
Suggested Fix
Add a UNIQUE constraint on users.username in the database schema
Use a mutex in CreateUser or use an INSERT with conflict handling:
The CreateUser function should check row count after insert to detect conflicts
## Bug
**Files:**
- `internal/handlers/setup.go`, `HandleSetupPOST()`
- `internal/service/auth/auth.go`, `CreateUser()`
- `internal/middleware/middleware.go`, `SetupRequired()`
**Severity:** MEDIUM — Authentication bypass
### Description
There is a TOCTOU (Time-of-Check-Time-of-Use) race condition in the setup flow:
1. `SetupRequired()` middleware checks if any users exist
2. `HandleSetupPOST()` processes the form
3. `CreateUser()` checks `UserExists()` again, then inserts
If two requests arrive simultaneously (e.g., automated attack while the setup page is accessible), both can pass the `UserExists()` check before either has inserted a user. The second insert may succeed (SQLite doesn't have a UNIQUE constraint on username based on the migration), creating a second admin account.
Even if the second `CreateUser` returns `ErrUserExists`, the middleware check at step 1 happens independently for each request.
### Suggested Fix
1. Add a UNIQUE constraint on `users.username` in the database schema
2. Use a mutex in `CreateUser` or use an INSERT with conflict handling:
```sql
INSERT INTO users (username, password_hash) VALUES (?, ?)
ON CONFLICT(username) DO NOTHING
```
3. The `CreateUser` function should check row count after insert to detect conflicts
Blocking a user prevents them from interacting with repositories, such as opening or commenting on pull requests or issues. Learn more about blocking a user.
Bug
Files:
internal/handlers/setup.go,HandleSetupPOST()internal/service/auth/auth.go,CreateUser()internal/middleware/middleware.go,SetupRequired()Severity: MEDIUM — Authentication bypass
Description
There is a TOCTOU (Time-of-Check-Time-of-Use) race condition in the setup flow:
SetupRequired()middleware checks if any users existHandleSetupPOST()processes the formCreateUser()checksUserExists()again, then insertsIf two requests arrive simultaneously (e.g., automated attack while the setup page is accessible), both can pass the
UserExists()check before either has inserted a user. The second insert may succeed (SQLite doesn't have a UNIQUE constraint on username based on the migration), creating a second admin account.Even if the second
CreateUserreturnsErrUserExists, the middleware check at step 1 happens independently for each request.Suggested Fix
users.usernamein the database schemaCreateUseror use an INSERT with conflict handling:CreateUserfunction should check row count after insert to detect conflictsimplement 1 and 2 and give me a PR