Skip to content

crypto.Sign panics for some inputs #269

Closed
@fjl

Description

@fjl

There are two problems, both of which are related to this change. Note that the change is not mentioned in the commit message.

One problem is that package crypto uses privateKey.D.Bytes() to marshal the private key. The value returned by Bytes will not have length 32 in all cases because math/big returns the minimum number of bytes required to hold the number.

I think we should pad the key with zeroes.

The other problem is that the data being signed cannot be larger than 32 bytes, because
nonce would be accessed with an index > 31.
If it is smaller than 32 bytes, the remaining bytes of the nonce will be the rest of the private key.

The parameter for the data is called hash in package crypto. I think we should document that the value
for this parameter must have length 32 and check that it does in the Sign function.

Crash:

Panic at i=534.
Key: (len=31) [139 238 11 176 202 152 178 4 145 65 239 166 32 93 76 229 172 233 51 179 54 143 168 210 31 183 235 209 38 188 56]
runtime error: index out of range
goroutine 1 [running]:
main.func·001()
    /Users/fjl/develop/eth/src/github.com/ethereum/go-ethereum/ktest.go:31 +0x3d2
github.com/obscuren/secp256k1-go.Sign(0xc20809deb8, 0x20, 0x20, 0xc20801fc01, 0x1f, 0x1f, 0x0, 0x0, 0x0, 0x0, ...)
    /Users/fjl/develop/eth/src/github.com/obscuren/secp256k1-go/secp256.go:130 +0x492
github.com/ethereum/go-ethereum/crypto.Sign(0xc20809deb8, 0x20, 0x20, 0xc208097770, 0x0, 0x0, 0x0, 0x0, 0x0)
    /Users/fjl/develop/eth/src/github.com/ethereum/go-ethereum/crypto/crypto.go:108 +0xc0
main.main()
    /Users/fjl/develop/eth/src/github.com/ethereum/go-ethereum/ktest.go:42 +0x306
exit status 1

Repro tool:

import (
    "crypto/ecdsa"
    "crypto/rand"
    "fmt"
    "os"
    "runtime"

    "github.com/ethereum/go-ethereum/crypto"
)

func main() {
    var (
        hash = make([]byte, 32)
        k    *ecdsa.PrivateKey
        i    int
        err  error
    )
    defer func() {
        if err := recover(); err != nil {
            fmt.Printf("Panic at i=%d.\n", i)
            if k != nil {
                enc := k.D.Bytes() // this is what package crypto does to marshal the key
                fmt.Printf("Key: (len=%d) %v\n", len(enc), enc)
            }
            fmt.Println(err)
            sb := make([]byte, 1000)
            fmt.Print(string(sb[:runtime.Stack(sb, false)]))
            os.Exit(1)
        }
    }()

    for ; i < 10000; i++ {
        k, err = ecdsa.GenerateKey(crypto.S256(), rand.Reader)
        if err != nil {
            fmt.Println("cannot generate key:", err)
            os.Exit(1)
        }
        crypto.Sign(hash, k)
    }
}

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