RQ-2426: sandbox rule code in an isolated-vm isolate (RCE fix)#109
Draft
dinex-dev wants to merge 1 commit into
Draft
RQ-2426: sandbox rule code in an isolated-vm isolate (RCE fix)#109dinex-dev wants to merge 1 commit into
dinex-dev wants to merge 1 commit into
Conversation
"Code"-type Modify Request/Response rules ran rule-supplied JS via
new Function(...) directly in the proxy's Node process — full require/process/
fs/child_process. Since code rules travel between users (shared lists, export/
import, team sync) with no sanitization, this was a supply-chain RCE: import +
enable a foreign rule and arbitrary native code runs as the victim.
- Run rule code inside an isolated-vm V8 isolate: no host realm (no require/
process/fs), hard 5s timeout, 128MB memory cap.
- Cross only copied data: args and $sharedState (ExternalCopy), read $sharedState
back and write it to GlobalStateProvider — same contract as before.
- Explicitly bridge the safe capabilities rules relied on: console (-> ctx.rq
.consoleLogs, same {type,args} shape), atob/btoa (host Buffer), and fetch
(async host reference; 10s timeout, 10MB response cap, forwards method/headers/
body; Response-like object with status/ok/headers.get()/text()/json()).
- Replace getFunctionFromString (in-process eval) with isValidFunctionString
(compile-only parse check in a throwaway isolate); update both processors to
validate then pass the source string (also fixes the request vs response
call-site inconsistency).
- Add isolated-vm@6.1.2; rebuild dist.
Verified via standalone harness: require/process/constructor-escape blocked
(even with fetch present), infinite loop times out, and JSON transforms, async/
promise, console, atob/btoa, $sharedState write-back, and fetch all work.
Integration notes: isolated-vm is native -> needs electron-rebuild for the
Electron 27 renderer (ABI 118). Remaining parity gaps (no Buffer/timers/URL/
TextEncoder) — add bridges if real rules need them.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
commit: |
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
1 participant
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.
Security fix for RQ-2426 (Critical) — supply-chain RCE.
Problem
"Code"-type Modify Request/Response rules ran rule-supplied JS via
new Function(...)directly in the proxy's Node process (fullrequire/process/fs/child_process). Code rules travel verbatim via shared lists, import/export, and team sync — so importing + enabling a foreign rule = arbitrary native code as the victim.Fix
Run rule code inside an
isolated-vmV8 isolate: no host realm, hard 5s timeout, 128MB cap. Only copied data crosses (args,$sharedState); narrow bridged shims preserve the existing contract:console,atob/btoa, andfetch(async, 10s/10MB bounded).getFunctionFromString(eval) →isValidFunctionString(compile-only check). Both processors pass the source string.Verified
Standalone harness (11/11 core + 4/4 fetch):
require/process/constructor-escape blocked even with fetch present; infinite loop times out; JSON transforms, async, console, base64,$sharedStatewrite-back, and fetch all preserved.isolated-vmneedselectron-rebuild(Electron ABI) before it runs in the desktop background renderer.Buffer/timers/URL/TextEncoderin the isolate — add bridges if real rules need them.Draft — pending review.