Skip to content

psbt: Fix PSBTInputSignedAndVerified bounds assert #34272

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

Jump to bottom
Merged
achow101 merged 1 commit into from
Jan 14, 2026
Merged

psbt: Fix PSBTInputSignedAndVerified bounds assert #34272

achow101 merged 1 commit into from
Jan 14, 2026

Conversation

@l0rinc
Copy link
Contributor

@l0rinc l0rinc commented Jan 13, 2026

This PR fixes an off-by-one in a debug assertion in PSBTInputSignedAndVerified.
The function indexes psbt.inputs[input_index], so the assertion must not allow indexing at psbt.inputs.size().

Found during review: #31650 (comment)

@l0rinc
psbt: Fix PSBTInputSignedAndVerified bounds assert
2f5b1c5 
The previous `assert` used `>=`, allowing `input_index == psbt.inputs.size()` and out-of-bounds access in `psbt.inputs[input_index]`.

Found during review: bitcoin#31650 (comment)
@DrahtBot DrahtBot added the PSBT label Jan 13, 2026
@DrahtBot
Copy link
Contributor

DrahtBot commented Jan 13, 2026 •
edited
Loading

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Code Coverage & Benchmarks

For details see: https://corecheck.dev/bitcoin/bitcoin/pulls/34272.

Reviews

See the guideline for information on the review process.

Type Reviewers
ACK optout21, maflcko, achow101

If your review is incorrectly listed, please copy-paste <!--meta-tag:bot-skip--> into the comment that the bot should ignore.

@optout21
Copy link
Contributor

optout21 commented Jan 13, 2026

utACK 2f5b1c5
Trivial change to an assert, can be reliably assessed by local code inspection (2 lines).
Code reviewed; build & unit tests verified locally.

@maflcko
Copy link
Member

maflcko commented Jan 13, 2026

lgtm ACK 2f5b1c5

This is just a refactor/doc change, because the UB can not be reached in the current code-base, and is assumed to be unreachable anyway (due to the use of assert)

@willcl-ark
Copy link
Member

willcl-ark commented Jan 13, 2026

FWIW I think I hit this on the fuzz tests here: https://github.com/willcl-ark/bitcoin/actions/runs/20883835381/job/60004100089#step:10:5614 So it may be possible for this to cause CI failure, at least.

@maflcko
Copy link
Member

maflcko commented Jan 13, 2026

@willcl-ark No, that is #33999 from the input fuzz_corpora/psbt/3fa30f92df4e391124a56b76cc3db3eb71b5d69c from commit bitcoin-core/qa-assets@00c335c

@willcl-ark
Copy link
Member

willcl-ark commented Jan 13, 2026

@willcl-ark No, that is #33999 from the input fuzz_corpora/psbt/3fa30f92df4e391124a56b76cc3db3eb71b5d69c from commit bitcoin-core/qa-assets@00c335c

ah ok i see, thanks.

@achow101
Copy link
Member

achow101 commented Jan 14, 2026

ACK 2f5b1c5

@achow101 achow101 merged commit 57350c5 into bitcoin:master Jan 14, 2026
27 checks passed
fanquake pushed a commit to fanquake/bitcoin that referenced this pull request Jan 14, 2026
@l0rinc @fanquake
psbt: Fix PSBTInputSignedAndVerified bounds assert
7e1090f 
The previous `assert` used `>=`, allowing `input_index == psbt.inputs.size()` and out-of-bounds access in `psbt.inputs[input_index]`.

Found during review: bitcoin#31650 (comment)

Github-Pull: bitcoin#34272
Rebased-From: 2f5b1c5
@fanquake fanquake mentioned this pull request Jan 14, 2026
Draft
@fanquake
Copy link
Member

fanquake commented Jan 14, 2026

Backported to 30.x in #34283.

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

Reviewers

No reviews

Assignees

No one assigned

Labels

PSBT Refactoring

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

7 participants

@l0rinc @DrahtBot @optout21 @maflcko @willcl-ark @achow101 @fanquake