Skip to content

[VUL-78] Add IP-based lockout to login endpoint#18947

Merged
jvcalderon merged 12 commits into
masterfrom
feature/vul-78-hardening-measure-sheets
Jun 10, 2026
Merged

[VUL-78] Add IP-based lockout to login endpoint#18947
jvcalderon merged 12 commits into
masterfrom
feature/vul-78-hardening-measure-sheets

Conversation

@jvcalderon

@jvcalderon jvcalderon commented Jun 9, 2026

Copy link
Copy Markdown
Contributor

What

Adds IP-based rate limiting to the login endpoint (POST /api/global/auth/:tenantId/login), complementing the existing per-email lockout introduced earlier on this branch.

Why

A security audit (Budibase/vulns#78) demonstrated that the login endpoint had no restriction on authentication attempts, allowing an attacker to run an automated brute-force attack (829 requests, no blocking). The CVSS score is 6.5 (AV:N/AC:L/PR:N/UI:N/S:U/C:L/I:L/A:N).

The email-based lockout already present on this branch protects accounts whose email address is known. The IP lockout adds a second layer that limits volume from any single source regardless of which accounts are targeted.

Screen recording

Screen.Recording.2026-06-09.at.11.43.08.mov

Addresses

https://github.com/Budibase/vulns/issues/78


Summary by cubic

Adds IP-based lockout to POST /api/global/auth/:tenantId/login to block brute-force attempts from a single source. Addresses VUL-78 by returning 429 with Retry-After and using an atomic Redis counter with TTL refreshed on each increment.

  • New Features

    • Added ipLockout middleware that counts attempts per IP with cache.increment (atomic), refreshes TTL (LOGIN_LOCKOUT_SECONDS) on each request, and responds with 429 and Retry-After once the limit is exceeded.
    • Runs ipLockout before emailLockout on the login route to avoid unnecessary DB lookups; added unit/integration tests.
    • Introduced LOGIN_IP_LOCKOUT_LIMIT env var (default 10).
  • Refactors

    • Added increment in @budibase/backend-core and re-exported as generic.increment.
    • Renamed lockout.ts to emailLockout.ts and updated imports/tests.

Written for commit c92b573. Summary will update on new commits.

Review in cubic

@github-actions github-actions Bot added the size/m label Jun 9, 2026
@jvcalderon jvcalderon self-assigned this Jun 9, 2026
@jvcalderon jvcalderon marked this pull request as ready for review June 9, 2026 12:08

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

2 issues found across 8 files

Confidence score: 3/5

  • There is some merge risk because the highest-severity finding is concrete and high-confidence: in packages/worker/src/middleware/ipLockout.ts, the non-atomic IP lockout counter update can let concurrent requests bypass rate limits.
  • In packages/worker/src/api/routes/global/auth.ts, evaluating email lockout before IP lockout means blocked IP traffic can still trigger email-related DB lookups, increasing unnecessary load and weakening the intended short-circuit behavior.
  • Pay close attention to packages/worker/src/middleware/ipLockout.ts and packages/worker/src/api/routes/global/auth.ts - fix lockout atomicity and enforce IP lockout earlier to prevent bypass and avoid extra DB work.

Reply with feedback, questions, or to request a fix.

Fix all with cubic | Re-trigger cubic

Comment thread packages/worker/src/middleware/ipLockout.ts Outdated
Comment thread packages/worker/src/api/routes/global/auth.ts Outdated
Comment thread packages/worker/src/environment.ts
IP lockout only requires a Redis read, while email lockout performs a
database lookup. Checking the IP first avoids the DB query for requests
that would be rejected anyway.
30 attempts per 15-minute window was too permissive for a brute-force
protection mechanism. 10 is still generous enough for legitimate use
behind corporate NATs while meaningfully limiting automated attacks.
Replace the non-atomic get/store pattern with cache.increment(), which
maps to Redis INCR. Concurrent requests could previously read the same
counter value and write back the same incremented result, bypassing the
limit. INCR is atomic so each request is counted exactly once.

TTL is set via EXPIRE only on the first increment (count === 1), which
is the standard Redis rate-limiting pattern.

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

1 issue found across 5 files (changes from recent commits).

Tip: Review your code locally with the cubic CLI to iterate faster.

Fix all with cubic | Re-trigger cubic

Comment thread packages/backend-core/src/cache/base/index.ts Outdated

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No issues found across 10 files

Confidence score: 5/5

  • Automated review surfaced no issues in the provided summaries.
  • No files require special attention.

Re-trigger cubic

@jvcalderon jvcalderon enabled auto-merge June 10, 2026 13:20
@jvcalderon jvcalderon merged commit cfcd5d3 into master Jun 10, 2026
34 checks passed
@jvcalderon jvcalderon deleted the feature/vul-78-hardening-measure-sheets branch June 10, 2026 13:21
@github-actions github-actions Bot locked and limited conversation to collaborators Jun 10, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants