Skip to content

fix(p2ms): fix signature validation bypass due to operator precedence#2333

Open
deepview-autofix wants to merge 1 commit intobitcoinjs:masterfrom
deepview-autofix:deepview/a1bfa1016b
Open

fix(p2ms): fix signature validation bypass due to operator precedence#2333
deepview-autofix wants to merge 1 commit intobitcoinjs:masterfrom
deepview-autofix:deepview/a1bfa1016b

Conversation

@deepview-autofix
Copy link
Copy Markdown

The expression (opts.allowIncomplete && x === OPS.OP_0) !== undefined
always evaluated to true when allowIncomplete was set, because
!== undefined was applied to the boolean result of && (which is
never undefined). This caused isAcceptableSignature() to accept any
arbitrary buffer as a valid signature when allowIncomplete was enabled,
completely bypassing signature validation.

Fixed by replacing !== undefined with !! to correctly coerce the
result to boolean without altering the intended logic: accept only
canonical script signatures or OP_0 placeholders (when allowIncomplete).

The expression `(opts.allowIncomplete && x === OPS.OP_0) !== undefined`
always evaluated to `true` when `allowIncomplete` was set, because
`!== undefined` was applied to the boolean result of `&&` (which is
never `undefined`). This caused `isAcceptableSignature()` to accept any
arbitrary buffer as a valid signature when `allowIncomplete` was enabled,
completely bypassing signature validation.

Fixed by replacing `!== undefined` with `!!` to correctly coerce the
result to boolean without altering the intended logic: accept only
canonical script signatures or OP_0 placeholders (when allowIncomplete).

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: DeepView Autofix <276251120+deepview-autofix@users.noreply.github.com>
Co-Authored-By: Nikita Skovoroda <chalkerx@gmail.com>
Signed-off-by: Nikita Skovoroda <chalkerx@gmail.com>
Copy link
Copy Markdown

@kewdex kewdex left a comment

Choose a reason for hiding this comment

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

utACK

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.

2 participants