Panic in btcec.RecoverCompact

[janos]

Calling btcec.RecoverCompact with btcec.S256() and certain signatures and hashes results in a panic.

I am providing a test btcrecover.zip with a case as an example that can be reproduced. Panic is a result of big int mod inverse returning nil in invr := new(big.Int).ModInverse(sig.R, curve.Params().N) github.com/btcsuite/btcd/btcec/signature.go:319 v0.21.0-beta.

Test execution result:

=== RUN   TestRecoverCompact_panic
=== RUN   TestRecoverCompact_panic/success
=== RUN   TestRecoverCompact_panic/failure
    /Users/janos/btcrecover/btcrecover_test.go:35: panic: runtime error: invalid memory address or nil pointer dereference
        goroutine 20 [running]:
        runtime/debug.Stack(0xc00009bab8, 0x115a7c0, 0x139a1d0)
        	/usr/local/go/src/runtime/debug/stack.go:24 +0x9f
        btcrecover.TestRecoverCompact_panic.func1.1(0xc000082c00)
        	/Users/janos/btcrecover/btcrecover_test.go:35 +0x57
        panic(0x115a7c0, 0x139a1d0)
        	/usr/local/go/src/runtime/panic.go:969 +0x175
        math/big.(*Int).Mul(0xc00009bda8, 0x0, 0xc00009bea8, 0x0)
        	/usr/local/go/src/math/big/int.go:168 +0xd0
        github.com/btcsuite/btcd/btcec.recoverKeyFromSignature(0x13a22e0, 0xc00009be98, 0xc0000b4100, 0x20, 0x20, 0x2, 0xc0000d6200, 0x1, 0x8, 0x0)
        	/Users/janos/go/pkg/mod/github.com/btcsuite/btcd@v0.21.0-beta/btcec/signature.go:322 +0x287
        github.com/btcsuite/btcd/btcec.RecoverCompact(0x13a22e0, 0xc0000ec050, 0x41, 0x41, 0xc0000b4100, 0x20, 0x20, 0x2e918628, 0x2e9186280000000f, 0x5f86c94b, ...)
        	/Users/janos/go/pkg/mod/github.com/btcsuite/btcd@v0.21.0-beta/btcec/signature.go:413 +0x251
        btcrecover.TestRecoverCompact_panic.func1(0xc000082c00)
        	/Users/janos/btcrecover/btcrecover_test.go:39 +0xa5
        testing.tRunner(0xc000082c00, 0xc00008e6c0)
        	/usr/local/go/src/testing/testing.go:1127 +0xef
        created by testing.(*T).Run
        	/usr/local/go/src/testing/testing.go:1178 +0x386
--- FAIL: TestRecoverCompact_panic (0.02s)
    --- PASS: TestRecoverCompact_panic/success (0.02s)
    --- FAIL: TestRecoverCompact_panic/failure (0.00s)
FAIL
FAIL	btcrecover	0.620s
FAIL

[onyb]

@janos I am trying to reproduce the error with your examples.

For the success case, can you confirm that it's normal that RecoverCompact returns the error "calculated Rx is larger than curve P" (with no panic)? For the failure case, I get a panic, so ACK.

[janos]

@janos I am trying to reproduce the error with your examples.

For the success case, can you confirm that it's normal that RecoverCompact returns the error "calculated Rx is larger than curve P" (with no panic)? For the failure case, I get a panic, so ACK.

Hi, @onyb. Thanks for looking into this issue. Yes, I confirm that the error you mentioned is a normal behaviour.

[onyb]

@janos I just opened a PR addressing the issue.

Note that the error is no longer going to be "calculated Rx is larger than curve P". It'll be "signature R is 0" for both the test cases, which is more accurate.

[janos]

Wonderful, thanks @onyb! The PR looks neat.

[jcvernaleo]

Fixed in #1691