fix(remote-control): validate tunnel base64 linearly to avoid V8 regexp stack overflow - #3709
Conversation
🦋 Changeset detectedLatest commit: 916af7f The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
commit: |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: df223c6023
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| "@moonshot-ai/kimi-code": patch | ||
| --- | ||
|
|
||
| Fix Remote Control tunnel uploads larger than ~3.5MB always failing with a 400: the base64 tunnel message is now validated in linear time instead of a backtracking regular expression that overflowed the V8 regexp stack, and internal tunnel errors now map to 502 instead of being disguised as 400. |
There was a problem hiding this comment.
When this changeset is consumed for a release, its implementation details about regex backtracking, V8 stack overflow, base64 validation strategy, and HTTP status mapping will be published in the user-facing changelog. Reduce it to the observable fix, such as stating that Remote Control now supports uploads larger than ~3.5 MB, as the repository explicitly requires one short sentence without causal or implementation clauses.
AGENTS.md reference: AGENTS.md:L86-L88
Useful? React with 👍 / 👎.
| (c >= 0x30 && c <= 0x39) || | ||
| c === 0x2b || | ||
| c === 0x2f || | ||
| (c === 0x3d && i >= value.length - 2); |
There was a problem hiding this comment.
When the relay sends a malformed final quartet such as ab=c, the penultimate = passes this check and the following c is also accepted as an alphabet character. Node's permissive Buffer.from(..., 'base64') then silently decodes a truncated payload, so data that the previous validator rejected is forwarded instead of receiving a 400 response; once padding is encountered, require every remaining character to be padding and enforce the valid one- or two-character padding forms.
Useful? React with 👍 / 👎.
df223c6 to
ae236d0
Compare
…xp stack overflow The Remote Control HTTP tunnel validated each base64 message with a backtracking regular expression. Requests larger than ~3.5MB (base64 ~5MB) exhausted the V8 Irregexp backtracking stack and threw RangeError, which the catch-all mapped to a 400 with an empty body, surfacing as a generic connection failure on the web client. Replace the regex with an O(n) character check, reject oversize requests from the base64 length before decoding, and map non-SyntaxError failures to 502 instead of 400.
ae236d0 to
916af7f
Compare
1 participant
Related Issue
Reported internally with a full root-cause analysis: Remote Control tunnel uploads larger than ~3.5MB always fail with HTTP 400 and an empty response body.
Problem
When uploading a file larger than ~3.5MB through the Remote Control web page (e.g. a 4.3MB image), the request always fails: the relay logs
status:400with an empty body and the web client reports a generic "cannot connect" error. Smaller uploads work.The Remote Control HTTP tunnel client validates every base64 tunnel message with a backtracking regular expression (
/^(?:[A-Za-z0-9+/]{4})*(?:[A-Za-z0-9+/]{2}==|[A-Za-z0-9+/]{3}=)?$/). V8's Irregexp engine implements the(?:...)*group with backtracking, pushing one backtrack point per iteration; a ~5.8MB base64 message (a 4.3MB request) needs ~1.45M iterations and exhausts the regexp backtracking stack, throwingRangeError: Maximum call stack size exceededon completely valid input. The catch-all inhandleHttpMessagethen maps every error tobuildErrorResponse(400)with an empty body, so a server-side bug is disguised as a client error. LANkimi webis unaffected because it never goes through the tunnel path.What changed
decodeBase64now validates input with an O(n) character check (length mod 4 plus per-character code-point validation) instead of the backtracking regex, so arbitrarily large valid base64 messages no longer crash. Invalid input still throws the sameSyntaxError.handleHttpMessagecatch now mapsSyntaxErrorto 400 and any other (internal) error to 502, matching the existing semantics offorwardHttpRequest, instead of reporting everything as 400.