clawwork-ai / clawwork-ai/ClawWork

[Bug] safeFetch buffers entire response before enforcing maxSize — memory DoS

Open Beginner friendly
#408 11 comments 0 reactions 0 assignees View on GitHub
area/dx kind/bug security
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

Open the contributing 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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.