Skip to content

fix(standalone): harden input validation on siteKey, siteverify body,…#258

Open
dvynshuu wants to merge 5 commits into
tiagozip:mainfrom
dvynshuu:fix/harden-input-validation
Open

fix(standalone): harden input validation on siteKey, siteverify body,…#258
dvynshuu wants to merge 5 commits into
tiagozip:mainfrom
dvynshuu:fix/harden-input-validation

Conversation

@dvynshuu
Copy link
Copy Markdown

Description

This PR introduces three focused input validation and defense-in-depth improvements to the standalone server to harden its security posture.

1. Added schema validation to /siteverify

The siteverify.js endpoint was the only route missing Elysia's t.Object body schema validation. Because it relied on raw destructuring (const { secret, response } = body;), passing non-string payloads (e.g., arrays or objects) would cause unhandled TypeError exceptions (e.g., response.split is not a function), potentially leading to unexpected crashes.

  • Fix: Added standard Elysia t.Object body and params validation to guarantee correct types before the handler executes.

2. Guarded siteKey path params against format manipulation

The siteKey path parameter is used extensively in direct Redis key construction (e.g., key:${siteKey}). Previously, any arbitrary string was accepted as a siteKey parameter.

  • Fix: Added a regex constraint (/^[a-f0-9]{10}$/) via onBeforeHandle to ensure that only properly formatted 10-character hex keys (matching the randomBytes(5) generation logic) can hit the database. This eliminates any risk of key namespace pollution or Redis protocol injection.

3. Fixed fallthrough bug in unblock-ip

In server.js, the block-ip handler correctly returns a 400 error if an unknown type is passed. However, the unblock-ip handler silently fell through and treated unknown types as an ip (key = body.ip;).

  • Fix: Aligned unblock-ip with block-ip to return a 400 Bad Request for invalid block types, preventing accidental unblocking of the wrong entries.

Type of Change

  • Bug fix (non-breaking change which fixes an issue)
  • Security improvement

Checklist

  • I have tested these changes locally
  • I have run biome check and fixed all warnings/errors

dvynshuu added 2 commits May 16, 2026 20:13
… and unblock-ip

- Add regex guard (^[a-f0-9]{10}$) on siteKey path params in cap routes to prevent malformed keys from reaching Redis

- Add Elysia t.Object body schema to siteverify endpoint (was the only endpoint missing type validation, could crash on non-string input)

- Fix unblock-ip handler to return 400 for invalid block types instead of silently falling through (matching block-ip behavior)
@dvynshuu dvynshuu closed this May 16, 2026
@dvynshuu dvynshuu reopened this May 16, 2026
- The test 'returns 404 for unknown site key' was failing in CI because it used the string 'nonexistent_key', which failed the new regex validation and returned 400 instead of 404. Changed to use a compliant 10-hex-char string (0000000000) so it bypasses the regex check and successfully hits the Redis query to test the 404 code path.
@tiagozip
Copy link
Copy Markdown
Owner

do you have benchmark results for that regex? is it vulnerable to redos?

@lligirlburg-wq
Copy link
Copy Markdown

lligirlburg-wq commented May 16, 2026 via email

@dvynshuu
Copy link
Copy Markdown
Author

No benchmark results were included in the PR initially, but I wrote a quick test to measure it against Bun's engine.

To answer your second question first: No, it is mathematically impossible for this regex to be vulnerable to ReDoS.

ReDoS vulnerabilities require a regex engine to perform exponential backtracking. Backtracking only happens when a regex has overlapping alternations (e.g., (a|a)+) or overlapping unbounded quantifiers (e.g., (a+)+).

The regex /^[a-f0-9]{10}$/ contains:

^ and $ (strict bounds)
A single character class [a-f0-9]
A strict, bounded length quantifier {10}
Because there are no variable-length quantifiers (* or +) or nested groups, the evaluation is strictly O(1). The engine simply checks up to 10 characters and immediately passes or fails with zero backtracking.

Here are the benchmark results running 10,000,000 iterations for each case on Bun v1.1.20+ (which uses WebKit's incredibly fast RegExp engine under the hood). Even checking against a malicious string of 100,000 characters is virtually instantaneous because it fails instantly at the 11th character or end-of-string bound.

const SITE_KEY_RE = /^[a-f0-9]{10}$/;
const valid = "a1b2c3d4e5";
const invalidShort = "a1b2c3d4e";
const invalidLong = "a1b2c3d4e5f6";
const maliciousString = "a".repeat(100_000); // Test for ReDoS
console.time("Valid 10-char hex string");
for(let i = 0; i < 10_000_000; i++) SITE_KEY_RE.test(valid);
console.timeEnd("Valid 10-char hex string");
console.time("Invalid short string");
for(let i = 0; i < 10_000_000; i++) SITE_KEY_RE.test(invalidShort);
console.timeEnd("Invalid short string");
console.time("Invalid long string");
for(let i = 0; i < 10_000_000; i++) SITE_KEY_RE.test(invalidLong);
console.timeEnd("Invalid long string");
console.time("Malicious string (100k chars)");
for(let i = 0; i < 10_000_000; i++) SITE_KEY_RE.test(maliciousString);
console.timeEnd("Malicious string (100k chars)");

Results (10 million iterations each):

[339.88ms] Valid 10-char hex string
[210.67ms] Invalid short string
[305.19ms] Invalid long string
[299.39ms] Malicious string (100k chars)

@tiagozip
Copy link
Copy Markdown
Owner

on 1, did you add an optional catch for the old schema?

@dvynshuu
Copy link
Copy Markdown
Author

Yes, good catch! I just pushed a follow-up commit to fix that.

@tiagozip
Copy link
Copy Markdown
Owner

uh no, that's not what i meant. on previous cap standalone versions, a key id parameter was also required. this would block everyone still sending a key id.

@dvynshuu
Copy link
Copy Markdown
Author

Ah, I see exactly what you mean! Because Elysia strictly validates objects and strips/rejects unknown properties by default, the new schema would have thrown a 400 Validation Error if anyone passed legacy fields like keyId in the JSON body.
I completely missed that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants