Threats tagged 'ghsa-x8rm-f576-gq67'

## Summary The regular expression behind `GraphQLPhoneNumber` since 1.24.1 backtracks exponentially. A string made of a run of digits followed by one character that makes the match fail (for example `"1".repeat(30) + "x"`) blocks the JavaScript thread for seconds, and every additional digit doubles the time. graphql-js calls `parseLiteral` while *validating* a document and `parseValue` while coercing variables, so the regex runs for every request that merely contains such a value in a `PhoneNumber` position, before execution and before any authentication or authorization step of the server. One small unauthenticated request is enough to stall a server process. ## Affected versions - `graphql-scalars` >= 1.24.1, <= 2.0.0 (2.0.0 is the current `latest`; the default branch still has the same pattern). - Not affected: 1.24.0 and the earlier versions I checked (1.22.4, 1.23.0), which use the linear `/^\+[1-9]\d{6,14}$/`. - Introduced by #2719 (c7f58da16, "PhoneNumber scalar is not fully compatible with E.164 format so updated regex to support phone number with extension"), released as 1.24.1 on 2025-01-29. The payload was ran against 1.22.4, 1.23.0, 1.24.0, 1.24.1, 1.24.2, 1.25.0, 1.26.0 and 2.0.0: the first three reject it instantly, the other five show the exponential growth. ## Root cause `src/scalars/PhoneNumber.ts`: ```js const PHONE_NUMBER_REGEX = /^\+?\d{1,3}(\-|\x20)?\(?\d+\)?((\-|\x20)?\d+)+$/; ``` In `\d+\)?((\-|\x20)?\d+)+$` the separator is optional, so a run of n digits can be split across the inner `\d+` and the outer `+` iterations in a number of ways that is exponential in n. When the overall match fails, for instance on a trailing non-digit, the engine tries all of them. ## Proof of concept 1. The scalar alone (`npm i graphql graphql-scalars@2.0.0`): ```js import { GraphQLPhoneNumber } from 'graphql-scalars'; for (const ok of ['+41791234567', '079 123 45 67', '+1 (123) 456-7890', 'nope']) { try { GraphQLPhoneNumber.parseValue(ok); } catch {} } for (const digits of [24, 26, 28, 30]) { const start = performance.now(); try { GraphQLPhoneNumber.parseValue('1'.repeat(digits) + 'x'); } catch {} console.log(`${digits} digits: ${((performance.now() - start) / 1000).toFixed(2)} s`); } ``` Measured warm, i.e. after ordinary inputs have been validated like in a running server: | digits | Node 22.19.0 | Node 26.10.0 | |-------:|-------------:|-------------:| | 24 | 0.10 s | 0.07 s | | 26 | 0.41 s | 0.29 s | | 28 | 1.63 s | 1.14 s | | 30 | 6.59 s | 4.62 s | The time doubles with every digit. The first execution of the regex in a fresh process is about 5x slower because V8 interprets a regexp on its first run (Node 22.19.0, 28 digits: 7.8 s). 2. Through graphql-js, no server and no authentication: a schema with a single `echo(phone: PhoneNumber)` field, 26 digits + `x`, Node 22.19.0, warm: | | literal (`parseLiteral`, validation) | variable (`parseValue`, coercion) | |---|---:|---:| | graphql 16.14.2 | 0.41 s | 0.43 s | | graphql 17.0.2 | 0.41 s | 0.83 s (evaluated twice) | 3. Also reproduced end to end against a GraphQL Yoga server: an anonymous HTTP request with a 27-digit value plus one invalid character stalled the whole process (a concurrent trivial `{ __typename }` request waited as long). A mutation that requires authentication was affected equally when called anonymously, because document validation runs before any authorization in resolvers or context. ## Impact Denial of service. Any GraphQL server that exposes `PhoneNumber` on an argument, an input object field or a variable can be stalled by a request of about 30 characters; roughly 40 characters would keep a process busy for hours (extrapolated from the doubling, not measured). `serialize` uses the same regex, so unvalidated stored values returned through a `PhoneNumber` field are affected too. The actual severity depends on the scalar being applied to untrusted input, which is the typical case. ## Suggested fix An equivalent pattern without ambiguous splits: ```js const PHONE_NUMBER_REGEX = /^\+?\d{1,3}[- ]?\(?\d(?:\d*\))?(?:[- ]?\d)+$/; ``` It accepts exactly the same strings as the current one, so there is no behaviour change: with `N = (S?\d+)+ = (S?\d)+` a digit run is just repeated single-digit iterations, hence `\d+\)?N = \d(\d*\))?N`. Verification: - exhaustive comparison of old and new pattern on every string up to length 9 over `1 + - space ( ) x`: 47,079,208 strings (1,832 accepted), 0 differences; - every sequence of up to 6 tokens from `1 11 111 1111 11111 + - space ( ) x`: 1,948,717 strings (100,380 accepted), 0 differences; - adversarial inputs (digit runs plus a failing character, digit/space and digit/dash alternations, parenthesis variants) scale linearly: 1,000,000 characters take about 10 ms, where the current pattern needs 0.3 s for 26. Optionally also reject values above a sane length before matching; the grammar currently has no upper bound and E.164 allows at most 15 digits. Above roughly