core/types: fix and test handling of faulty nil-returning signer (#28879)

This adds an error if the signer returns a nil value for one of the signature value fields.
This commit is contained in:
protolambda 2024-01-27 14:16:20 -06:00 committed by GitHub
parent bc0b87ca19
commit 2e947b7a00
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
3 changed files with 64 additions and 2 deletions

View File

@ -19,6 +19,7 @@ package types
import ( import (
"bytes" "bytes"
"errors" "errors"
"fmt"
"io" "io"
"math/big" "math/big"
"sync/atomic" "sync/atomic"
@ -320,6 +321,7 @@ func (tx *Transaction) Cost() *big.Int {
// RawSignatureValues returns the V, R, S signature values of the transaction. // RawSignatureValues returns the V, R, S signature values of the transaction.
// The return values should not be modified by the caller. // The return values should not be modified by the caller.
// The return values may be nil or zero, if the transaction is unsigned.
func (tx *Transaction) RawSignatureValues() (v, r, s *big.Int) { func (tx *Transaction) RawSignatureValues() (v, r, s *big.Int) {
return tx.inner.rawSignatureValues() return tx.inner.rawSignatureValues()
} }
@ -508,6 +510,9 @@ func (tx *Transaction) WithSignature(signer Signer, sig []byte) (*Transaction, e
if err != nil { if err != nil {
return nil, err return nil, err
} }
if r == nil || s == nil || v == nil {
return nil, fmt.Errorf("%w: r: %s, s: %s, v: %s", ErrInvalidSig, r, s, v)
}
cpy := tx.inner.copy() cpy := tx.inner.copy()
cpy.setSignatureValues(signer.ChainID(), v, r, s) cpy.setSignatureValues(signer.ChainID(), v, r, s)
return &Transaction{inner: cpy, time: tx.time}, nil return &Transaction{inner: cpy, time: tx.time}, nil

View File

@ -18,11 +18,13 @@ package types
import ( import (
"errors" "errors"
"fmt"
"math/big" "math/big"
"testing" "testing"
"github.com/ethereum/go-ethereum/common" "github.com/ethereum/go-ethereum/common"
"github.com/ethereum/go-ethereum/crypto" "github.com/ethereum/go-ethereum/crypto"
"github.com/ethereum/go-ethereum/params"
"github.com/ethereum/go-ethereum/rlp" "github.com/ethereum/go-ethereum/rlp"
) )
@ -136,3 +138,53 @@ func TestChainId(t *testing.T) {
t.Error("expected no error") t.Error("expected no error")
} }
} }
type nilSigner struct {
v, r, s *big.Int
Signer
}
func (ns *nilSigner) SignatureValues(tx *Transaction, sig []byte) (r, s, v *big.Int, err error) {
return ns.v, ns.r, ns.s, nil
}
// TestNilSigner ensures a faulty Signer implementation does not result in nil signature values or panics.
func TestNilSigner(t *testing.T) {
key, _ := crypto.GenerateKey()
innerSigner := LatestSignerForChainID(big.NewInt(1))
for i, signer := range []Signer{
&nilSigner{v: nil, r: nil, s: nil, Signer: innerSigner},
&nilSigner{v: big.NewInt(1), r: big.NewInt(1), s: nil, Signer: innerSigner},
&nilSigner{v: big.NewInt(1), r: nil, s: big.NewInt(1), Signer: innerSigner},
&nilSigner{v: nil, r: big.NewInt(1), s: big.NewInt(1), Signer: innerSigner},
} {
t.Run(fmt.Sprintf("signer_%d", i), func(t *testing.T) {
t.Run("legacy", func(t *testing.T) {
legacyTx := createTestLegacyTxInner()
_, err := SignNewTx(key, signer, legacyTx)
if !errors.Is(err, ErrInvalidSig) {
t.Fatal("expected signature values error, no nil result or panic")
}
})
// test Blob tx specifically, since the signature value types changed
t.Run("blobtx", func(t *testing.T) {
blobtx := createEmptyBlobTxInner(false)
_, err := SignNewTx(key, signer, blobtx)
if !errors.Is(err, ErrInvalidSig) {
t.Fatal("expected signature values error, no nil result or panic")
}
})
})
}
}
func createTestLegacyTxInner() *LegacyTx {
return &LegacyTx{
Nonce: uint64(0),
To: nil,
Value: big.NewInt(0),
Gas: params.TxGas,
GasPrice: big.NewInt(params.GWei),
Data: nil,
}
}

View File

@ -65,6 +65,12 @@ var (
) )
func createEmptyBlobTx(key *ecdsa.PrivateKey, withSidecar bool) *Transaction { func createEmptyBlobTx(key *ecdsa.PrivateKey, withSidecar bool) *Transaction {
blobtx := createEmptyBlobTxInner(withSidecar)
signer := NewCancunSigner(blobtx.ChainID.ToBig())
return MustSignNewTx(key, signer, blobtx)
}
func createEmptyBlobTxInner(withSidecar bool) *BlobTx {
sidecar := &BlobTxSidecar{ sidecar := &BlobTxSidecar{
Blobs: []kzg4844.Blob{emptyBlob}, Blobs: []kzg4844.Blob{emptyBlob},
Commitments: []kzg4844.Commitment{emptyBlobCommit}, Commitments: []kzg4844.Commitment{emptyBlobCommit},
@ -85,6 +91,5 @@ func createEmptyBlobTx(key *ecdsa.PrivateKey, withSidecar bool) *Transaction {
if withSidecar { if withSidecar {
blobtx.Sidecar = sidecar blobtx.Sidecar = sidecar
} }
signer := NewCancunSigner(blobtx.ChainID.ToBig()) return blobtx
return MustSignNewTx(key, signer, blobtx)
} }