Skip to content

fix(canonicalize): preserve nested __proto__ keys in signing bytes (closes #51)#11

Open
akuraposo wants to merge 1 commit into
permission-protocol:mainfrom
akuraposo:fix/proto-pollution-canonicalize
Open

fix(canonicalize): preserve nested __proto__ keys in signing bytes (closes #51)#11
akuraposo wants to merge 1 commit into
permission-protocol:mainfrom
akuraposo:fix/proto-pollution-canonicalize

Conversation

@akuraposo
Copy link
Copy Markdown

Summary

Fixes #51: the receipt canonicalizer rebuilt nested objects with a
plain {} target, so a nested signed JSON object containing an own
__proto__ key was reassigned through Object.prototype's setter
instead of creating an own property. The key was then silently
omitted from the canonical JSON covered by the Ed25519 signature.

As a result, a nested requestJson.__proto__ value could be
changed after signing without changing the canonical bytes, and the
modified receipt would still pass pp verify.

Fix

  • Inner sortKeys target is now Object.create(null), with field
    copy via Object.defineProperty. A nested __proto__ key is
    treated as an own enumerable string-keyed property of the result.
  • Outer canonical object in canonicalizeReceipt uses the same
    null-prototype + defineProperty pattern, so the defense applies
    at the top level too.

Validation

  • npm test -- --run — all 6 tests pass (5 pre-existing + 1 new
    regression test that forges a requestJson with a nested own
    __proto__ key, mutates its value, and asserts the canonical
    bytes change)
  • Confirmed the new test FAILS on the pre-fix code and PASSES on
    the patched code
  • npm run build — clean
  • git diff --check — clean

Bounty

Submitted for assessment under #36 as a distinct Ed25519
verification-flow flaw. Payout details can be provided privately
after validation.

…loses #51)

The receipt canonicalizer rebuilt nested objects with a plain `{}`
target, so a nested signed JSON object containing an own
`__proto__` key was reassigned through `Object.prototype`'s setter
instead of creating an own property. The key was then silently omitted
from the canonical JSON covered by the Ed25519 signature.

As a result, a nested `requestJson.__proto__` value could be changed
after signing without changing the canonical bytes, and the modified
receipt would still pass `pp verify`.

This patch:

- Builds the inner sorted object with `Object.create(null)` and
  copies fields via `Object.defineProperty`, so a nested `__proto__`
  key is treated as an own enumerable string-keyed property of the
  result.
- Builds the outer `canonical` object with the same null-prototype
  + `defineProperty` pattern, so the same defense applies at the
  top level.
- Adds a regression test that forges a `requestJson` with a nested
  own `__proto__` key, mutates its value, and asserts the canonical
  bytes change.

All 6 tests pass (5 pre-existing + 1 new). `npm run build` clean.

Closes #51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant