Skip to content

BTCSignPSBT panics with nil pointer dereference when a non-Taproot input lacks non_witness_utxo #186

Description

@xrviv

Device.BTCSignPSBT panics instead of returning an error when a PSBT input has witness_utxo but no non_witness_utxo and the firmware requests the previous transaction.

Reproduction (master 8bccc8b, simulators v9.19.0 and v9.27.1): take the locktime-zero vector from api/firmware/testdata/btc-transaction-test-vectors.json, set packet.Inputs[0].NonWitnessUtxo = nil, call BTCSignPSBT(BTCCoin_TBTC, packet, nil).

panic: runtime error: invalid memory address or nil pointer dereference
  (*Device).nonAtomicBTCSign  api/firmware/btc.go:493   (prevtx.Version)
  (*Device).BTCSign            api/firmware/btc.go:580
  (*Device).BTCSignPSBT        api/firmware/psbt.go:622

newBTCTxFromPSBT leaves BTCTxInput.PrevTx nil in this case, and the PREVTX_INIT / PREVTX_INPUT / PREVTX_OUTPUT handlers at btc.go:487-514 dereference it without a check. BTCSignNeedsNonWitnessUTXOs correctly returns true beforehand, so this is only hit by callers that skip that helper, but a returned error would be friendlier than a panic. A similar unchecked index exists at psbt.go:456 (NonWitnessUtxo.TxOut[prevOutPoint.Index]) for a malformed PSBT.

No security impact: the device has not signed anything at that point, and it verifies the streamed prevtx hash and amounts before signing.

Suggested fix: return an error from the three handlers when PrevTx == nil, and bounds-check the outpoint index. A community fork carrying such a patch exists at 4xvgal/bitbox02-api-go@44800a0.

Activity

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

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions