fix(canonicalize): preserve nested __proto__ keys in signing bytes (closes #51)#11
Open
akuraposo wants to merge 1 commit into
Open
Conversation
…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
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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 throughObject.prototype's setterinstead 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 bechanged after signing without changing the canonical bytes, and the
modified receipt would still pass
pp verify.Fix
Object.create(null), with fieldcopy via
Object.defineProperty. A nested__proto__key istreated as an own enumerable string-keyed property of the result.
canonicalobject incanonicalizeReceiptuses the samenull-prototype +
definePropertypattern, so the defense appliesat the top level too.
Validation
npm test -- --run— all 6 tests pass (5 pre-existing + 1 newregression test that forges a
requestJsonwith a nested own__proto__key, mutates its value, and asserts the canonicalbytes change)
the patched code
npm run build— cleangit diff --check— cleanBounty
Submitted for assessment under #36 as a distinct Ed25519
verification-flow flaw. Payout details can be provided privately
after validation.