Jelajahi Sumber

fix(code-runtime-python): drop fd-3 frames with illegal UTF-8 instead of mangling them

The reviewer's standing issue: line.toString('utf8') silently replaces illegal
bytes with U+FFFD, so a forged frame could land a corrupted completion value
(the honest child's lossless encoder never emits non-UTF-8, so such a frame is
hostile traffic). The fd-3 frame decode now uses a fatal UTF-8 decoder: an
illegal byte throws and the frame is dropped, same treatment as the
unsafe-integer check. A forged illegal-UTF-8 done frame is verified to be
dropped (the run settles on the program's real return), and reverting to
toString makes the case fail.
Chinesezjc 1 bulan lalu
induk
melakukan
6bdbe71092

+ 18 - 1
packages/code-runtime/code-runtime-python/src/index.ts

@@ -429,6 +429,11 @@ const TRUNCATION_MARKER = '… [truncated]'
  * The ellipsis is 3 bytes, so this is 15, not the string's 13 code units.
  */
 const TRUNCATION_MARKER_BYTES = Buffer.byteLength(TRUNCATION_MARKER, 'utf8')
+// Fatal UTF-8 decoder for fd-3 frames: `toString('utf8')` replaces illegal
+// bytes with U+FFFD, which would silently corrupt a completion or binding
+// payload a forged frame smuggled in; a fatal decode throws instead and the
+// frame is dropped. Non-stream mode keeps it stateless across lines.
+const UTF8_FATAL = new TextDecoder('utf-8', { fatal: true })
 
 /**
  * Serialized JSON byte width of one character, given its code point and the
@@ -1422,7 +1427,19 @@ export class PythonCodeRuntime extends CodeRuntime {
             // reject any frame past FRAME_PARSE_CAP_BYTES before this join, so
             // every line in this loop is within the cap by construction — a
             // per-line check would be dead code.
-            const text = line.toString('utf8')
+            // `toString('utf8')` would silently REPLACE illegal bytes with
+            // U+FFFD, corrupting a completion or binding payload a forged
+            // frame smuggled in (the honest child's lossless encoder never
+            // emits non-UTF-8, so such a frame is hostile traffic). The fatal
+            // decode throws on them and the frame is dropped — not accepted
+            // with a mangled value — the same treatment as the unsafe-integer
+            // check below.
+            let text: string
+            try {
+              text = UTF8_FATAL.decode(line)
+            } catch {
+              continue
+            }
             // JSON.parse would silently ROUND an integer token outside the
             // safe range before validation could see it, so a forged frame
             // could smuggle a corrupted value into a dispatch or completion.

+ 18 - 0
packages/code-runtime/code-runtime-python/tests/runtime.spec.ts

@@ -1916,6 +1916,24 @@ describe('PythonCodeRuntime — programs and bindings', () => {
     expect(result.logs).toEqual(['committed', logTruncationMarker(64)])
   }, 15_000)
 
+  it('drops a forged fd-3 frame with illegal UTF-8 instead of accepting a mangled value', async () => {
+    // toString('utf8') would replace the illegal 0xFF with U+FFFD, so a forged
+    // done frame could land a corrupted completion value; the fatal decode
+    // throws and the frame is dropped. The program's real return still settles
+    // the run with the honest value.
+    const { runtime } = await setup()
+    const result = await runtime.run({
+      program: [
+        'import os',
+        "os.write(3, b'{\"type\":\"done\",\"value\":\"bad' + bytes([0xFF]) + b'\"}\\n')",
+        'return "ok"',
+      ].join('\n'),
+      bindings: [],
+    })
+    expect(result.error).toBeUndefined()
+    expect(result.value).toBe('ok')
+  }, 15_000)
+
   it('no-ops a closing frame once an open flood already truncated the ledger', async () => {
     // The closing-frame branch's post-truncation arm: an open flood exhausts
     // the ledger (logsTruncated set, marker pushed), then a closing frame