Repository navigation
fix(security): rate-limit, proxy-trust and origin hardening - #79
Merged
Merged
Conversation
store.set() called client.set(k, v, 'PX', ttl). node-redis does not understand the positional form and stores the key with no expiry, so on node-redis every withBan ban marker had no TTL, read back as already expired, and was never enforced — while the key lived forever. set() now goes through the shared setWithTTL, which dispatches per driver. The live suite covers set and withBan on ioredis and node-redis.
Several ways to get a spoofed client IP out of getClientIp: - X-Real-IP was read whenever X-Forwarded-For was absent, though most proxies never set or strip it - only the first X-Forwarded-For header line was read, so a client could put its own line first - proxyCount counted forwarded entries only, one hop off from Express's trust proxy: N, and a non-number silently fell through to the left-most (client-controlled) entry - ::ffff:-mapped peers never matched an IPv4 trust list The chain is now [socket peer, ...X-Forwarded-For right-to-left] across every header line, IPv4-mapped addresses are folded, proxyCount means exactly what trust proxy: N means (0 = the socket peer) and must be a non-negative integer, and X-Real-IP is only used when named in headers. BREAKING CHANGE: proxyCount is now Express-compatible (previously proxyCount: N behaved like trust proxy: N + 1), and X-Real-IP is no longer read by default.
webhookVerify and webhookVerifyStripe guarded the secret with !secret, which is false for Buffer.alloc(0). An empty key (an unset env var read into a Buffer) was accepted, and an HMAC under an empty key can be computed by anyone. Both now require a non-empty string or Buffer.
Three gaps in the rate-limit middleware: - a request with no derivable key (no client IP, or keyGenerator returning nothing) skipped limiting entirely - the default key was the raw IPv6 address, so one host rotating through its /64 got 2^64 fresh buckets - the documented trustProxy option was never read Unattributable requests now share one bucket (onMissingKey: 'shared', default) or are refused with 429 (onMissingKey: 'deny'); IPv6 clients are keyed by their /64 (ipv6Subnet, configurable); IPv4-mapped IPv6 is keyed as IPv4. Passing trustProxy now throws — proxy trust belongs to the framework (req.ip / request.ip), which is what the key uses. BREAKING CHANGE: unattributable requests are limited instead of passing, IPv6 keys change to /64 prefixes, and rateLimit.trustProxy is rejected.
isSameOrigin compared URL.origin strings, and every opaque origin
(javascript:, data:, file:, custom schemes) serializes as 'null' — so
isSameOrigin('javascript:alert(1)', 'myapp://cb') returned true. Either
side being opaque now means not the same origin.
cors() and checkOrigin() called RegExp.test() on the caller's pattern directly. With the g or y flag set, lastIndex carries over between calls, so the same allowed origin alternately passed and failed. Patterns are now compiled once without g / y, and a pattern with no $ end anchor (which also matches origins with anything appended) emits a one-time EXORTEK_SECURITY_UNANCHORED_ORIGIN warning. A CORS denial from an allowlist also carries Vary: Origin, so a shared cache cannot replay it to (or a grant from) another origin.
The safe-method set was rebuilt from options.safeMethods on every request. The default set is now a module constant and custom lists are memoised per array.
The Fastify CSRF hook ran in onRequest, before the body is parsed, so the form-field fallback never saw a token and form posts without an x-csrf-token header always failed. CSRF now runs in preHandler, after body parsing, in both csrfPlugin and the umbrella securityPlugin.
The signed double-submit token is not bound to a session; the default __Host-csrf cookie name is what stops an attacker on a sibling subdomain (or plain http) from planting their own valid cookie + token pair. Renaming the cookie silently dropped that protection. The CSRF middleware now emits EXORTEK_SECURITY_CSRF_COOKIE_PREFIX when the name lacks __Host- and points at generateForSession for session binding.
tokenBucket and leakyBucket wrote their state back on every call, including denials. Each write re-quantized the fractional refill to milli-tokens and moved updatedAt forward, so a client polling every millisecond rounded its way to more refill than configured — about 1.6x the allowed rate at refillRate 0.6/s (100 allowed in 100 s instead of ~61). A denial now leaves the state untouched.
sanitizeParams copied keys with out[key] = value, so an own __proto__ key in the parsed query (JSON bodies, qs with allowPrototypes) replaced the result's prototype and injected inherited properties such as isAdmin. It now skips __proto__ / constructor / prototype, matching sanitizeBody.
Web docs for the Express-compatible getClientIp proxyCount, the X-Forwarded-For-only default, the rate-limit middleware's onMissingKey / ipv6Subnet options (and the removed trustProxy), anchored CORS / origin patterns, the CSRF __Host- prefix warning and opaque origins in isSameOrigin.
This was referenced Oct 24, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What / why
Rate-limit, proxy-trust and origin hardening for
@exortek/security. Each fix comes with a regression test that reproduced the problem.set()used the ioredis-only positional'PX', ttlform; node-redis stored the ban marker with no TTL, read it back as expired, and never enforced it. Now dispatched per driver, with live tests on ioredis and node-redis.getClientIpspoofing.X-Real-IPwas trusted whenever XFF was missing, only the first XFF header line was read,proxyCountwas one hop off from Express'strust proxy: N, a non-number fell through to the client-controlled left-most entry, and::ffff:peers never matched an IPv4 trust list. It now walks[socket peer, ...XFF right-to-left]exactly like Express.trustProxyoption was never read. Now: shared bucket (or 429 withonMissingKey: 'deny'),/64keys for IPv6, andtrustProxythrows.isSameOrigin; stateless + anchored-warning origin RegExps andVary: Originon CORS denials; Fastify CSRF reads the body token (preHandler);sanitizeParamsdrops prototype-pollution keys; the CSRF middleware warns when the cookie loses__Host-.Which package(s)?
@exortek/securityType of change
Checklist
yarn verifypasses locally, including the Redis integration suites on ioredis and node-redisweb/content/security/…)@exortek/securitymajor with a migration sectionAdditional notes
Independent of #78 (no shared files). Not released on merge; ships with the rest of the hardening PRs.