Skip to content

fix(psbt): filter nil entries from taproot fields before sort/serialize#2511

Open
ThomsenDrake wants to merge 2 commits intobtcsuite:masterfrom
ThomsenDrake:fix-psbt-nil-panic-2495
Open

fix(psbt): filter nil entries from taproot fields before sort/serialize#2511
ThomsenDrake wants to merge 2 commits intobtcsuite:masterfrom
ThomsenDrake:fix-psbt-nil-panic-2495

Conversation

@ThomsenDrake
Copy link
Copy Markdown

Fixes #2495

Problem

panics with a nil pointer dereference when , , or slices contain nil entries. The sort comparison functions and serialization loops dereference slice elements without nil checks.

Fix

Filter nil elements from all three taproot-specific slice fields before sorting and serialization. This is a defensive approach: nil entries are silently removed rather than rejected with an error, matching the behavior of silently ignoring zero-value fields that PSBT already uses elsewhere.

Changes

  • btcutil/psbt/partial_input.go: Added nil-filtering blocks before sort/serialize for TaprootScriptSpendSig, TaprootLeafScript, and TaprootBip32Derivation.

Testing

All existing btcutil/psbt tests pass (go test ./btcutil/psbt/...).

…ze (fixes btcsuite#2495)

B64Encode panics with nil pointer dereference when TaprootLeafScript,
TaprootScriptSpendSig, or TaprootBip32Derivation slices contain nil
entries. Filter nil elements before sorting and serialization to
prevent the panic.
@TechLateef
Copy link
Copy Markdown

Thanks for fixing this crash issue! The defensive filtering approach makes sense and matches PSBT's pattern of silently handling edge cases.

One suggestion for cleaner code: The filtering logic is repeated 3 times with just type differences. Consider extracting to a helper function:

func filterNil[T any](slice []*T) []*T {
filtered := make([]*T, 0, len(slice))
for _, item := range slice {
if item != nil {
filtered = append(filtered, item)
}
}
return filtered
}

Then simplify to:
pi.TaprootScriptSpendSig = filterNil(pi.TaprootScriptSpendSig)
pi.TaprootLeafScript = filterNil(pi.TaprootLeafScript)
pi.TaprootBip32Derivation = filterNil(pi.TaprootBip32Derivation)

@ThomsenDrake
Copy link
Copy Markdown
Author

Thanks for the review, @TechLateef! Great suggestion on extracting a helper function. I'll refactor the repeated filtering logic into a generic helper. I also noticed the same nil-filtering pattern likely applies to the deserialize path where these slices are built up — I'll audit that as well in the next push.

Addresses review feedback from @TechLateef (btcsuite#2511). The repeated
nil-filtering logic (3 blocks) is now consolidated into a single
generic helper function filterNil[T any], reducing duplication
and improving readability.

No behavioral change — the filtering logic is identical.
@ThomsenDrake
Copy link
Copy Markdown
Author

Pushed the refactoring as discussed. Extracted a filterNil[T any] generic helper that consolidates the three repeated nil-filtering blocks into a single reusable function. Net result: -30 lines, +17 lines. All existing tests pass.

I also audited the deserialize path — nil entries can only originate from external code that manually constructs PInput with nil slice elements, since deserialization always appends concrete pointer values. So the serialize-side filter is the correct and sufficient fix.

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.

[bug]: pstb.Packet.B64Encode panics when given a nil TaprootScriptLeaf entry

2 participants