clawwork-ai / clawwork-ai/ClawWork
[Bug] safeFetch buffers entire response before enforcing maxSize — memory DoS
- Dominant language
- TypeScript
- Stars
- 532
- Forks
- 75
- Avg merge
- 5h 31m
- Merged PRs (30d)
- 1
Description
## Problem
`safeFetch` enforces `maxSize` in two places, but both checks happen **after** the full response body has already been materialized in memory:
1. The `content-length` header check is advisory — a malicious server can omit or lie about it.
2. `await res.arrayBuffer()` buffers the **entire** response body into a single `ArrayBuffer` before the byte-length check runs. By the time we check `ab.byteLength > maxSize`, the memory has already been allocated.
A malicious image URL pointing to a server that omits `content-length` and streams an endlessly large body causes the main process to allocate gigabytes of memory before the check fires, leading to an OOM crash of the entire Electron app.
## Location
**File:** `packages/desktop/src/main/net/safe-fetch.ts:21-27`
```typescript
const res = await net.fetch(url, { signal: controller.signal });
if (!res.ok) throw new Error(`fetch ${url}: ${res.status}`);
const cl = Number(res.headers.get('content-length') ?? '0');
if (cl > maxSize) throw new Error('response too large');
const ab = await res.arrayBuffer(); // ← entire body buffered here
if (ab.byteLength > maxSize) throw new Error('response too large');
return Buffer.from(ab);
```
## Fix Approach
Switch to a streaming reader that enforces `maxSize` incrementally and aborts as soon as the limit is exceeded:
```typescript
const reader = res.body?.getReader();
if (!reader) throw new Error('no body');
const chunks: Uint8Array[] = [];
let total = 0;
while (true) {
const { done, value } = await reader.read();
if (done) break;
total += value.byteLength;
if (total > maxSize) {
controller.abort();
throw new Error('response too large');
}
chunks.push(value);
}
return Buffer.concat(chunks.map((c) => Buffer.from(c)));
```
Keep the initial `content-length` pre-check as a cheap short-circuit for well-behaved servers.
## Verification
1. Run `pnpm check` — must pass.
2. Unit test: mock `net.fetch` to return a body that streams more than `maxSize` bytes with no `content-length` header; assert `safeFetch` throws before allocating more than `maxSize` bytes.
3. Manual: point `safeFetch` at a test server that streams `/dev/urandom`; confirm memory usage stays bounded.
## Context
- **WG:** Observability & DX (security — memory DoS)
- **Priority:** Medium
- **Estimated effort:** 1 hour
Contributor guide
Research direction
Start in packages/desktop/src/main/net/safe-fetch.ts:21-27 and inspect the existing safeFetch flow and its callers. Keep the content-length pre-check, enforce the limit while reading the response, and add the described streaming-body unit test; run pnpm check to verify completion.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- electron, typescript
- Domain
- desktop, security
- Issue type
- Bug
- Difficulty
- 2/5
- Estimated time
- 1-3 hours
- Activity status
- Quiet
- Clarity
- Clearly specified
- Newbie friendliness
- 72/100