Sjors/libwally-swift

PSBT.finalize() causing a crash

Closed this issue ยท 6 comments

I have found a bug that is only effecting a very specific type of transaction, I am able to consistently replicate this.

Finalizing single signature PSBT's with more then two inputs are causing LbWally-Swift to crash here:

public mutating func finalize() -> Bool {
        var psbt = UnsafeMutablePointer<wally_psbt>.allocate(capacity: 1)
        psbt.initialize(to: self.wally_psbt)
        defer {
            psbt.deallocate()
        }
        guard wally_finalize_psbt(psbt) == WALLY_OK else {
            return false
        }
        return true
    }

Screen Shot 2020-04-21 at 11 59 12

Here is an example culprit PSBT:

cHNidP8BALACAAAAA4H3/ONLmdp/hMvfX/4K8HbeuxN1D1Cl5YxD/XOs0VJuAAAAAAD9////iU1RvtTUo4/2Sf4s9FBMphOVdKPiYajZc57+P+XpKegBAAAAAP3///+SVXjLEcHD16XjRJERJRugRJHAvkibNUQ684Q8/1j+OwEAAAAA/f///wEudAAAAAAAACIAIMNsUgb+DfidqgmB+jkSHBjNfTSYC3YXjt3Clr8rFn5jAAAAAAABAR8QJwAAAAAAABYAFLANUblgpSofl8eylI7CRPE62QzFIgICt4h4iljY72O3vP+cyWkfgw11OV7HHar8cmYOFtBn7SZHMEQCIAbIiwMClMlCr+6ye+iYn7w//DaGNFoQqxx9jYMft1+HAiAYGUu2t/iyix0waHQu6z0AE7w8e3UWwm56Z4r11rgZIAEiBgK3iHiKWNjvY7e8/5zJaR+DDXU5XscdqvxyZg4W0GftJhhJD/gBVAAAgAEAAIAAAACAAAAAAA0AAAAAAQEfECcAAAAAAAAWABRVOL2EqfdQecYd+gMRYno4ybOPAiICAoI5knNM8b7LjSQkzMoJZuEUfYQEtxCVOYSSnWyyOugWRzBEAiBXePGlvNZIQEHzwqkuGuuNWXjDwDaZfSb8NrCKqKSqPgIgb6VJmX61zQ7w3JmAoqRg/Aht29cxZ7xEI02UUe94TB0BIgYCgjmSc0zxvsuNJCTMyglm4RR9hAS3EJU5hJKdbLI66BYYSQ/4AVQAAIABAACAAAAAgAAAAAAMAAAAAAEBHxAnAAAAAAAAFgAUm1khoZrGf9yKKICK6hDsIuU5egQiAgNr3T6Q5LQBEdbt1MpH7wnC/1vlEXVBLP75YlsPkk/jOkcwRAIgKeakN8YyoxRy3cW1SseGxH4umXPdhgnHOkciMR0z4cwCICy7kmTQBEoQQ8Q7kHp+TD+62aTp8eo+AsQJl8s6TS/2ASIGA2vdPpDktAER1u3UykfvCcL/W+URdUEs/vliWw+ST+M6GEkP+AFUAACAAQAAgAAAAIAAAAAACwAAAAAA

I bypass LibWally-Swift and use Bitcoin Core to sign the PSBT without issue so I do not think there is anything wrong with the PSBT or its signatures.

When I setup a breakpoint and step through it the following error prints out on the crash point:

Printing description of psbt:
expression produced error: error: /var/folders/3d/jzth48_15_g1mrhgr2pkkwcm0000gn/T/expr16-427a63..swift:1:92: error: use of undeclared type 'CLibWally'
Swift._DebuggerSupport.stringForPrintObject(Swift.UnsafePointer<Swift.UnsafeMutablePointer<CLibWally.wally_psbt>>(bitPattern: 0x114c80e50)!.pointee)

Multisig transactions with any number of inputs works fine, single sig transactions less then 3 inputs works fine. Which is strange. Perhaps related to #30 ?

Thanks again for the great library. If there is anything else I can do to help solve the issue just let me know.

can confirm this is still occurring.

Hi,

I've added this PSBT as a test case on my current PR to libwally here: ElementsProject/libwally-core#211 - I can confirm it doesn't crash on the wally side.

The wally PR should be merged in a few days, @Sjors please see CHANGES.md for API changes in this update and be aware that the PSBT interface has changed significantly but can now be considered final and shouldn't change again.

Hi,

I've added this PSBT as a test case on my current PR to libwally here: ElementsProject/libwally-core#211 - I can confirm it doesn't crash on the wally side.

The wally PR should be merged in a few days, @Sjors please see CHANGES.md for API changes in this update and be aware that the PSBT interface has changed significantly but can now be considered final and shouldn't change again.

Thanks for the heads up. I see libwally is on version 0.7.8 but this library is still on 0.7.7, will it be getting updated? I think there has been numerous issues related to psbt fixed on libwally core that out to be brought over here.

The latest fixes from the PR linked above need to be merged in wally, and then any fallout from testing c-lightning fixed before we consider a 0.7.9 wally release. It should be fine to migrate to wally master once the PR is merged though.

All changes are now in wally master which should fix this crash. However note that ElementsProject/libwally-core#213 is not yet fixed which means PSBTs created with the latest version of core will not be signed correctly (this is not a regression, we just need to update to the new core changes around the segwit fee attack).

Sjors commented

I'm bumping the libwally-core dependency in #45.