Skip to content

fix(protocol): size check before JSON parse in receiveEnvelope (#49) #65

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
rfxlamia merged 2 commits into from
Jul 23, 2026
Merged

fix(protocol): size check before JSON parse in receiveEnvelope (#49) #65

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
df90c1b
fix(protocol): size check before JSON parse in receiveEnvelope
rfxlamia Jul 23, 2026
6d0390f
test(protocol): align step 1–6 wording in receiveEnvelope test
rfxlamia Jul 23, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 6 additions & 6 deletions packages/protocol/src/crypto/receive-envelope.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,17 +75,17 @@ export async function receiveEnvelope(
selfId: string,
deps: ReceiveEnvelopeDeps,
): Promise<ReceiveEnvelopeResult> {
// Step 0: known wire version
// Step 1 (§4.3): pre-decode wire size cap — before any JSON.parse
if (utf8ToBytes(wire).length > MAX_ENVELOPE_WIRE_BYTES) {
return { ok: false, error: "envelope_too_large" };
}

// §4.1 wire version gate (between §4.3 steps 1 and 2)
const outerVersion = parseOuterVersion(wire);
if (outerVersion !== null && outerVersion !== 1) {
return { ok: false, error: "unsupported_version" };
}

// Step 1: pre-decode wire size cap
if (utf8ToBytes(wire).length > MAX_ENVELOPE_WIRE_BYTES) {
return { ok: false, error: "envelope_too_large" };
}

// Step 2: strict-decode outer + parse body JSON
let outer: OuterEnvelope;
try {
Expand Down
24 changes: 21 additions & 3 deletions packages/protocol/src/receive-envelope.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -110,8 +110,8 @@ function tamperBody(wire: string, patch: (body: EnvelopeBody) => void): string {
return serializeOuterEnvelope(tampered);
}

describe("receiveEnvelope steps 0–6 (§4.3)", () => {
it("step 0: unknown wire version → unsupported_version, dispatch not called", async () => {
describe("receiveEnvelope steps 1–6 (§4.3)", () => {

@rfxlamia rfxlamia Jul 23, 2026

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[nit] The describe block was renamed to steps 1–6 (§4.3) to match the §4.3 numbering, but a later case in the same block still says valid wire passing steps 0–6 continues to step 7+ (line 307). After this PR there is no “step 0” in either the implementation comments or the suite naming, so the leftover “0” is inconsistent and can confuse anyone mapping tests to SPEC steps.

Suggestion: Rename that case to valid wire passing steps 1–6 continues to step 7+ (or similar) in a follow-up or this PR if you touch the file again.

it("§4.1 wire version: unknown version at valid size → unsupported_version, dispatch not called", async () => {
const { wire, bob, bobId } = makeValidWire();
const deps = makeDeps(bob);
const badWire = tamperOuter(wire, (outer) => {
Expand All @@ -137,6 +137,24 @@ describe("receiveEnvelope steps 0–6 (§4.3)", () => {
expect(deps.seqStore.commitAccepted).not.toHaveBeenCalled();
});

it("step 1: oversized wire with v:2 → envelope_too_large (not unsupported_version)", async () => {
const { wire, bob, bobId } = makeValidWire();
const deps = makeDeps(bob);
const badWire = padWireToSize(
tamperOuter(wire, (outer) => {
outer.v = 2;
}),
MAX_ENVELOPE_WIRE_BYTES + 1,
);
expect(wireUtf8Length(badWire)).toBeGreaterThan(MAX_ENVELOPE_WIRE_BYTES);

const result = await receiveEnvelope(badWire, bobId, deps);

expect(result).toEqual({ ok: false, error: "envelope_too_large" });
expect(deps.dispatch).not.toHaveBeenCalled();
expect(deps.seqStore.commitAccepted).not.toHaveBeenCalled();
});

it("step 1: wire === 65536 bytes passes size check", async () => {
const { wire, bob, bobId } = makeValidWire();
const deps = makeDeps(bob);
Expand Down Expand Up @@ -286,7 +304,7 @@ describe("receiveEnvelope steps 0–6 (§4.3)", () => {
expect(deps.seqStore.commitAccepted).not.toHaveBeenCalled();
});

it("valid wire passing steps 0–6 continues to step 7+ (not rejected early)", async () => {
it("valid wire passing steps 1–6 continues to step 7+ (not rejected early)", async () => {
const { wire, bob, bobId } = makeValidWire();
const deps = makeDeps(bob);

Expand Down
Loading