diff --git a/core/rawdb/chain_iterator.go b/core/rawdb/chain_iterator.go index c024cbf45..84f79de94 100644 --- a/core/rawdb/chain_iterator.go +++ b/core/rawdb/chain_iterator.go @@ -86,6 +86,7 @@ func InitDatabaseFromFreezer(db ethdb.Database) { type blockTxHashes struct { number uint64 hashes []common.Hash + err error } // iterateTransactions iterates over all transactions in the (canon) block @@ -144,7 +145,12 @@ func iterateTransactions(db ethdb.Database, from uint64, to uint64, reverse bool var body types.Body if err := rlp.DecodeBytes(data.rlp, &body); err != nil { log.Warn("Failed to decode block body", "block", data.number, "error", err) - return + select { + case hashesCh <- &blockTxHashes{number: data.number, err: err}: + case <-interrupt: + return + } + continue } var hashes []common.Hash for _, tx := range body.Transactions { @@ -211,6 +217,14 @@ func indexTransactions(db ethdb.Database, from uint64, to uint64, interrupt chan } // Next block available, pop it off and index it delivery := queue.PopItem().(*blockTxHashes) + if delivery.err != nil { + log.Warn("Missing or corrupt block body during indexing; aborting run", "block", delivery.number, "err", delivery.err) + WriteTxIndexTail(batch, lastNum) + if err := batch.Write(); err != nil { + log.Crit("Failed writing batch to db", "error", err) + } + return + } lastNum = delivery.number WriteTxLookupEntries(batch, delivery.number, delivery.hashes) blocks++ @@ -300,6 +314,10 @@ func unindexTransactions(db ethdb.Database, from uint64, to uint64, interrupt ch } delivery := queue.PopItem().(*blockTxHashes) nextNum = delivery.number + 1 + if delivery.err != nil { + log.Warn("Block skipped during unindexing; tx lookup entries NOT deleted", "block", delivery.number, "err", delivery.err) + continue + } DeleteTxLookupEntries(batch, delivery.hashes) txs += len(delivery.hashes) blocks++ diff --git a/core/rawdb/chain_iterator_test.go b/core/rawdb/chain_iterator_test.go index dfad1b963..d119f9768 100644 --- a/core/rawdb/chain_iterator_test.go +++ b/core/rawdb/chain_iterator_test.go @@ -206,3 +206,163 @@ func TestIndexTransactions(t *testing.T) { verify(8, 11, true, 8) verify(0, 8, false, 8) } + +func TestIndexTransactionsMissingBody(t *testing.T) { + chainDb := NewMemoryDatabase() + + var txs []*types.Transaction + to := common.BytesToAddress([]byte{0x11}) + + block := types.NewBlock(&types.Header{Number: big.NewInt(0)}, nil, nil, nil, newHasher()) + WriteBlock(chainDb, block) + WriteCanonicalHash(chainDb, block.Hash(), block.NumberU64()) + + for i := uint64(1); i <= 10; i++ { + tx := types.NewTx(&types.LegacyTx{ + Nonce: i, + GasPrice: big.NewInt(11111), + Gas: 1111, + To: &to, + Value: big.NewInt(111), + Data: []byte{0x11, 0x11, 0x11}, + }) + txs = append(txs, tx) + block = types.NewBlock(&types.Header{Number: big.NewInt(int64(i))}, []*types.Transaction{tx}, nil, nil, newHasher()) + WriteBlock(chainDb, block) + WriteCanonicalHash(chainDb, block.Hash(), block.NumberU64()) + } + + missingBlock := uint64(5) + hash := ReadCanonicalHash(chainDb, missingBlock) + DeleteBody(chainDb, hash, missingBlock) + + IndexTransactions(chainDb, 0, 11, nil) + + // Tail must not advance past the missing block: the last successfully + // indexed block from the top was 6, so tail == 6. + tail := ReadTxIndexTail(chainDb) + if tail == nil || *tail != missingBlock+1 { + t.Fatalf("tx index tail mismatch after index with missing body: got %v, want %d", tail, missingBlock+1) + } + // Blocks above the gap are indexed; blocks at and below are not. + for i := uint64(1); i <= 10; i++ { + number := ReadTxLookupEntry(chainDb, txs[i-1].Hash()) + if i > missingBlock { + if number == nil { + t.Fatalf("tx index %d missing after indexing (above gap)", i) + } + } else { + if number != nil { + t.Fatalf("tx index %d should not be indexed (at/below gap)", i) + } + } + } +} + +func TestUnindexTransactionsMissingBody(t *testing.T) { + chainDb := NewMemoryDatabase() + + var txs []*types.Transaction + to := common.BytesToAddress([]byte{0x11}) + + block := types.NewBlock(&types.Header{Number: big.NewInt(0)}, nil, nil, nil, newHasher()) + WriteBlock(chainDb, block) + WriteCanonicalHash(chainDb, block.Hash(), block.NumberU64()) + + for i := uint64(1); i <= 10; i++ { + tx := types.NewTx(&types.LegacyTx{ + Nonce: i, + GasPrice: big.NewInt(11111), + Gas: 1111, + To: &to, + Value: big.NewInt(111), + Data: []byte{0x11, 0x11, 0x11}, + }) + txs = append(txs, tx) + block = types.NewBlock(&types.Header{Number: big.NewInt(int64(i))}, []*types.Transaction{tx}, nil, nil, newHasher()) + WriteBlock(chainDb, block) + WriteCanonicalHash(chainDb, block.Hash(), block.NumberU64()) + } + IndexTransactions(chainDb, 0, 11, nil) + + for i := 1; i <= 10; i++ { + number := ReadTxLookupEntry(chainDb, txs[i-1].Hash()) + if number == nil { + t.Fatalf("tx index %d missing after indexing", i) + } + } + + missingBlock := uint64(5) + hash := ReadCanonicalHash(chainDb, missingBlock) + DeleteBody(chainDb, hash, missingBlock) + + UnindexTransactions(chainDb, 0, 11, nil) + + tail := ReadTxIndexTail(chainDb) + if tail == nil || *tail != 11 { + t.Fatalf("tx index tail mismatch after unindex with missing body: got %v, want 11", tail) + } + for i := 1; i <= 10; i++ { + number := ReadTxLookupEntry(chainDb, txs[i-1].Hash()) + if uint64(i) == missingBlock { + continue + } + if number != nil { + t.Fatalf("tx index %d should be deleted after unindexing", i) + } + } +} + +func TestUnindexTransactionsCorruptBody(t *testing.T) { + chainDb := NewMemoryDatabase() + + var txs []*types.Transaction + to := common.BytesToAddress([]byte{0x11}) + + block := types.NewBlock(&types.Header{Number: big.NewInt(0)}, nil, nil, nil, newHasher()) + WriteBlock(chainDb, block) + WriteCanonicalHash(chainDb, block.Hash(), block.NumberU64()) + + for i := uint64(1); i <= 10; i++ { + tx := types.NewTx(&types.LegacyTx{ + Nonce: i, + GasPrice: big.NewInt(11111), + Gas: 1111, + To: &to, + Value: big.NewInt(111), + Data: []byte{0x11, 0x11, 0x11}, + }) + txs = append(txs, tx) + block = types.NewBlock(&types.Header{Number: big.NewInt(int64(i))}, []*types.Transaction{tx}, nil, nil, newHasher()) + WriteBlock(chainDb, block) + WriteCanonicalHash(chainDb, block.Hash(), block.NumberU64()) + } + IndexTransactions(chainDb, 0, 11, nil) + + for i := 1; i <= 10; i++ { + number := ReadTxLookupEntry(chainDb, txs[i-1].Hash()) + if number == nil { + t.Fatalf("tx index %d missing after indexing", i) + } + } + + corruptBlock := uint64(5) + hash := ReadCanonicalHash(chainDb, corruptBlock) + WriteBodyRLP(chainDb, hash, corruptBlock, []byte{0xff}) + + UnindexTransactions(chainDb, 0, 11, nil) + + tail := ReadTxIndexTail(chainDb) + if tail == nil || *tail != 11 { + t.Fatalf("tx index tail mismatch after unindex with corrupt body: got %v, want 11", tail) + } + for i := 1; i <= 10; i++ { + number := ReadTxLookupEntry(chainDb, txs[i-1].Hash()) + if uint64(i) == corruptBlock { + continue + } + if number != nil { + t.Fatalf("tx index %d should be deleted after unindexing", i) + } + } +} diff --git a/core/types/transaction.go b/core/types/transaction.go index 74eda4c44..233bdc47f 100644 --- a/core/types/transaction.go +++ b/core/types/transaction.go @@ -380,7 +380,7 @@ func (tx *Transaction) FeeTokenID() uint16 { func (tx *Transaction) FeeLimit() *big.Int { if !tx.IsMorphTx() { - return big.NewInt(0) + return nil } return tx.AsMorphTx().FeeLimit } @@ -912,9 +912,9 @@ func (tx *Transaction) AsMessage(s Signer, baseFee *big.Int) (Message, error) { msg := Message{ nonce: tx.Nonce(), gasLimit: tx.Gas(), - gasPrice: new(big.Int).Set(tx.GasPrice()), - gasFeeCap: new(big.Int).Set(tx.GasFeeCap()), - gasTipCap: new(big.Int).Set(tx.GasTipCap()), + gasPrice: tx.GasPrice(), + gasFeeCap: tx.GasFeeCap(), + gasTipCap: tx.GasTipCap(), to: tx.To(), amount: tx.Value(), data: tx.Data(), @@ -936,8 +936,8 @@ func (tx *Transaction) AsMessage(s Signer, baseFee *big.Int) (Message, error) { return Message{}, err } - if tx.FeeLimit() != nil { - msg.feeLimit = tx.FeeLimit() + if fl := tx.FeeLimit(); fl != nil { + msg.feeLimit = new(big.Int).Set(fl) } var err error diff --git a/core/types/transaction_signing.go b/core/types/transaction_signing.go index 8204497ef..76f5c9579 100644 --- a/core/types/transaction_signing.go +++ b/core/types/transaction_signing.go @@ -286,7 +286,10 @@ func (s *modernSigner) SignatureValues(tx *Transaction, sig []byte) (R, S, V *bi if tx.inner.chainID().Sign() != 0 && tx.inner.chainID().Cmp(s.chainID) != 0 { return nil, nil, nil, fmt.Errorf("%w: have %d want %d", ErrInvalidChainId, tx.inner.chainID(), s.chainID) } - R, S, _ = decodeSignature(sig) + R, S, _, err = decodeSignature(sig) + if err != nil { + return nil, nil, nil, err + } V = big.NewInt(int64(sig[64])) return R, S, V, nil } @@ -383,7 +386,10 @@ func (s EIP155Signer) SignatureValues(tx *Transaction, sig []byte) (R, S, V *big if tx.Type() != LegacyTxType { return nil, nil, nil, ErrTxTypeNotSupported } - R, S, V = decodeSignature(sig) + R, S, V, err = decodeSignature(sig) + if err != nil { + return nil, nil, nil, err + } if s.chainId.Sign() != 0 { V = big.NewInt(int64(sig[64] + 35)) V.Add(V, s.chainIdMul) @@ -457,8 +463,8 @@ func (fs FrontierSigner) SignatureValues(tx *Transaction, sig []byte) (r, s, v * if tx.Type() != LegacyTxType { return nil, nil, nil, ErrTxTypeNotSupported } - r, s, v = decodeSignature(sig) - return r, s, v, nil + r, s, v, err = decodeSignature(sig) + return r, s, v, err } // Hash returns the hash to be signed by the sender. @@ -474,14 +480,14 @@ func (fs FrontierSigner) Hash(tx *Transaction) common.Hash { }) } -func decodeSignature(sig []byte) (r, s, v *big.Int) { +func decodeSignature(sig []byte) (r, s, v *big.Int, err error) { if len(sig) != crypto.SignatureLength { - panic(fmt.Sprintf("wrong size for signature: got %d, want %d", len(sig), crypto.SignatureLength)) + return nil, nil, nil, fmt.Errorf("wrong size for signature: got %d, want %d", len(sig), crypto.SignatureLength) } r = new(big.Int).SetBytes(sig[:32]) s = new(big.Int).SetBytes(sig[32:64]) v = new(big.Int).SetBytes([]byte{sig[64] + 27}) - return r, s, v + return r, s, v, nil } func recoverPlain(sighash common.Hash, R, S, Vb *big.Int, homestead bool) (common.Address, error) { diff --git a/core/types/transaction_signing_test.go b/core/types/transaction_signing_test.go index 2cf662456..e9b721049 100644 --- a/core/types/transaction_signing_test.go +++ b/core/types/transaction_signing_test.go @@ -136,3 +136,55 @@ func TestChainId(t *testing.T) { t.Error("expected no error") } } + +func TestSignatureValuesError(t *testing.T) { + tx := NewTransaction(0, common.Address{}, big.NewInt(0), 0, big.NewInt(0), nil) + signer := HomesteadSigner{} + + tests := []struct { + name string + sig []byte + }{ + {name: "empty", sig: nil}, + {name: "short", sig: make([]byte, 64)}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + defer func() { + if r := recover(); r != nil { + t.Fatalf("Panicked for invalid signature length, expected error: %v", r) + } + }() + _, err := tx.WithSignature(signer, tt.sig) + if err == nil { + t.Fatal("Expected error for invalid signature length, got nil") + } + t.Logf("Got expected error: %v", err) + }) + } +} + +func TestSignSetCode(t *testing.T) { + key, _ := defaultTestKey() + auth := SetCodeAuthorization{ + Address: common.HexToAddress("0x1234567890abcdef1234567890abcdef12345678"), + Nonce: 7, + } + + signed, err := SignSetCode(key, auth) + if err != nil { + t.Fatalf("SignSetCode returned error: %v", err) + } + if signed.Address != auth.Address { + t.Fatalf("address mismatch: have %s want %s", signed.Address, auth.Address) + } + if signed.Nonce != auth.Nonce { + t.Fatalf("nonce mismatch: have %d want %d", signed.Nonce, auth.Nonce) + } + if signed.R.IsZero() || signed.S.IsZero() { + t.Fatal("expected non-zero signature values") + } + if signed.V > 1 { + t.Fatalf("unexpected y parity: %d", signed.V) + } +} diff --git a/core/types/transaction_test.go b/core/types/transaction_test.go index 157d59928..fdd165c96 100644 --- a/core/types/transaction_test.go +++ b/core/types/transaction_test.go @@ -1513,3 +1513,65 @@ func compareMemoPtr(a, b *[]byte) bool { } return bytes.Equal(*a, *b) } + +func TestAsMessageFeeLimitIsolation(t *testing.T) { + key, _ := crypto.HexToECDSA("b71c71a67e1177ad4e901695e1b4b9ee17ae16c6668d313eac2f96dbcda3f291") + originalFeeLimit := big.NewInt(999) + signer := LatestSignerForChainID(common.Big1) + + tx, err := SignTx(NewTx(&MorphTx{ + ChainID: big.NewInt(1), + Nonce: 0, + GasTipCap: big.NewInt(1), + GasFeeCap: big.NewInt(10), + Gas: 21000, + Value: big.NewInt(0), + FeeTokenID: 1, + FeeLimit: new(big.Int).Set(originalFeeLimit), + }), signer, key) + if err != nil { + t.Fatal(err) + } + + msg, err := tx.AsMessage(signer, big.NewInt(1)) + if err != nil { + t.Fatal(err) + } + + if msg.FeeLimit() == nil { + t.Fatal("msg.FeeLimit() should not be nil for MorphTx") + } + if msg.FeeLimit().Cmp(originalFeeLimit) != 0 { + t.Fatalf("msg.FeeLimit() = %v, want %v", msg.FeeLimit(), originalFeeLimit) + } + + msg.FeeLimit().SetInt64(0) + + if tx.FeeLimit().Cmp(originalFeeLimit) != 0 { + t.Fatalf("modifying msg.feeLimit mutated original tx: got %v, want %v", tx.FeeLimit(), originalFeeLimit) + } +} + +func TestAsMessageNonMorphTxFeeLimitNil(t *testing.T) { + key, _ := crypto.HexToECDSA("b71c71a67e1177ad4e901695e1b4b9ee17ae16c6668d313eac2f96dbcda3f291") + signer := HomesteadSigner{} + + tx, err := SignTx(NewTx(&LegacyTx{ + Nonce: 0, + GasPrice: big.NewInt(1), + Gas: 21000, + Value: big.NewInt(0), + }), signer, key) + if err != nil { + t.Fatal(err) + } + + msg, err := tx.AsMessage(signer, nil) + if err != nil { + t.Fatal(err) + } + + if msg.FeeLimit() != nil { + t.Fatalf("non-MorphTx msg.FeeLimit() should be nil, got %v", msg.FeeLimit()) + } +} diff --git a/core/types/tx_setcode.go b/core/types/tx_setcode.go index c1725ddbf..fe9d813ba 100644 --- a/core/types/tx_setcode.go +++ b/core/types/tx_setcode.go @@ -94,7 +94,10 @@ func SignSetCode(prv *ecdsa.PrivateKey, auth SetCodeAuthorization) (SetCodeAutho if err != nil { return SetCodeAuthorization{}, err } - r, s, _ := decodeSignature(sig) + r, s, _, err := decodeSignature(sig) + if err != nil { + return SetCodeAuthorization{}, err + } return SetCodeAuthorization{ ChainID: auth.ChainID, Address: auth.Address, diff --git a/crypto/crypto_test.go b/crypto/crypto_test.go index 58747221f..d2db3326d 100644 --- a/crypto/crypto_test.go +++ b/crypto/crypto_test.go @@ -92,6 +92,32 @@ func TestUnmarshalPubkey(t *testing.T) { } } +func TestS256RejectsCoordinatesOutsideField(t *testing.T) { + curve := S256() + params := curve.Params() + + if !curve.IsOnCurve(params.Gx, params.Gy) { + t.Fatal("generator point should be on curve") + } + if curve.IsOnCurve(big.NewInt(-1), params.Gy) { + t.Fatal("negative x should not be on curve") + } + if curve.IsOnCurve(params.Gx, big.NewInt(-1)) { + t.Fatal("negative y should not be on curve") + } + if curve.IsOnCurve(params.P, params.Gy) { + t.Fatal("x = P should not be on curve") + } + if curve.IsOnCurve(params.Gx, params.P) { + t.Fatal("y = P should not be on curve") + } + + pPlus7 := new(big.Int).Add(params.P, big.NewInt(7)) + if curve.IsOnCurve(pPlus7, params.Gy) { + t.Fatal("x = P+7 should not be on curve") + } +} + func TestSign(t *testing.T) { key, _ := HexToECDSA(testPrivHex) addr := common.HexToAddress(testAddrHex) diff --git a/crypto/ecies/ecies.go b/crypto/ecies/ecies.go index 64b5a99d0..2a6f03b83 100644 --- a/crypto/ecies/ecies.go +++ b/crypto/ecies/ecies.go @@ -122,6 +122,9 @@ func (prv *PrivateKey) GenerateShared(pub *PublicKey, skLen, macLen int) (sk []b if prv.PublicKey.Curve != pub.Curve { return nil, ErrInvalidCurve } + if pub.X == nil || pub.Y == nil || !pub.Curve.IsOnCurve(pub.X, pub.Y) { + return nil, ErrInvalidPublicKey + } if skLen+macLen > MaxSharedKeyLength(pub) { return nil, ErrSharedKeyTooBig } @@ -285,7 +288,7 @@ func (prv *PrivateKey) Decrypt(c, s1, s2 []byte) (m []byte, err error) { switch c[0] { case 2, 3, 4: rLen = (prv.PublicKey.Curve.Params().BitSize + 7) / 4 - if len(c) < (rLen + hLen + 1) { + if len(c) < (rLen + hLen + params.BlockSize) { return nil, ErrInvalidMessage } default: @@ -301,6 +304,9 @@ func (prv *PrivateKey) Decrypt(c, s1, s2 []byte) (m []byte, err error) { if R.X == nil { return nil, ErrInvalidPublicKey } + if !R.Curve.IsOnCurve(R.X, R.Y) { + return nil, ErrInvalidPublicKey + } z, err := prv.GenerateShared(R, params.KeyLen, params.KeyLen) if err != nil { diff --git a/crypto/ecies/ecies_test.go b/crypto/ecies/ecies_test.go index 014f7974c..b023150a8 100644 --- a/crypto/ecies/ecies_test.go +++ b/crypto/ecies/ecies_test.go @@ -301,7 +301,7 @@ func testParamSelection(t *testing.T, c testCase) { params := ParamsFromCurve(c.Curve) if params == nil { t.Fatal("ParamsFromCurve returned nil") - } else if params != nil && !cmpParams(params, c.Expected) { + } else if !cmpParams(params, c.Expected) { t.Fatalf("ecies: parameters should be invalid (%s)\n", c.Name) } @@ -362,6 +362,26 @@ func TestBasicKeyValidation(t *testing.T) { } } +func TestDecryptRejectsCiphertextAtLegacyMinimumLength(t *testing.T) { + prv, err := GenerateKey(rand.Reader, DefaultCurve, nil) + if err != nil { + t.Fatal(err) + } + params, err := pubkeyParams(&prv.PublicKey) + if err != nil { + t.Fatal(err) + } + rLen := (prv.PublicKey.Curve.Params().BitSize + 7) / 4 + legacyMinLen := rLen + params.Hash().Size() + 1 + + ciphertext := make([]byte, legacyMinLen) + ciphertext[0] = 0x04 + + if _, err := prv.Decrypt(ciphertext, nil, nil); err != ErrInvalidMessage { + t.Fatalf("expected ErrInvalidMessage for legacy minimum ciphertext length, got %v", err) + } +} + func TestBox(t *testing.T) { prv1 := hexKey("4b50fa71f5c3eeb8fdc452224b2395af2fcc3d125e06c32c82e048c0559db03f") prv2 := hexKey("d0b043b4c5d657670778242d82d68a29d25d7d711127d17b8e299f156dad361a") @@ -413,6 +433,47 @@ func TestSharedKeyStatic(t *testing.T) { } } +func TestGenerateSharedInvalidPublicKey(t *testing.T) { + prv, err := GenerateKey(rand.Reader, DefaultCurve, nil) + if err != nil { + t.Fatal(err) + } + skLen := MaxSharedKeyLength(&prv.PublicKey) / 2 + + // Off-curve point: valid-looking coordinates that don't satisfy y² = x³ + 7 + offCurve := &PublicKey{ + Curve: DefaultCurve, + X: big.NewInt(1), + Y: big.NewInt(1), + Params: ParamsFromCurve(DefaultCurve), + } + if _, err := prv.GenerateShared(offCurve, skLen, skLen); err != ErrInvalidPublicKey { + t.Fatalf("expected ErrInvalidPublicKey for off-curve point, got %v", err) + } + + // Nil X coordinate + nilX := &PublicKey{ + Curve: DefaultCurve, + X: nil, + Y: big.NewInt(1), + Params: ParamsFromCurve(DefaultCurve), + } + if _, err := prv.GenerateShared(nilX, skLen, skLen); err != ErrInvalidPublicKey { + t.Fatalf("expected ErrInvalidPublicKey for nil X, got %v", err) + } + + // Nil Y coordinate + nilY := &PublicKey{ + Curve: DefaultCurve, + X: big.NewInt(1), + Y: nil, + Params: ParamsFromCurve(DefaultCurve), + } + if _, err := prv.GenerateShared(nilY, skLen, skLen); err != ErrInvalidPublicKey { + t.Fatalf("expected ErrInvalidPublicKey for nil Y, got %v", err) + } +} + func hexKey(prv string) *PrivateKey { key, err := crypto.HexToECDSA(prv) if err != nil { diff --git a/crypto/kzg4844/kzg4844_ckzg_cgo.go b/crypto/kzg4844/kzg4844_ckzg_cgo.go index 46509674b..5fd722c0a 100644 --- a/crypto/kzg4844/kzg4844_ckzg_cgo.go +++ b/crypto/kzg4844/kzg4844_ckzg_cgo.go @@ -25,7 +25,7 @@ import ( gokzg4844 "github.com/crate-crypto/go-eth-kzg" ckzg4844 "github.com/ethereum/c-kzg-4844/v2/bindings/go" - "github.com/ethereum/go-ethereum/common/hexutil" + "github.com/morph-l2/go-ethereum/common/hexutil" ) // ckzgAvailable signals whether the library was compiled into Geth. diff --git a/crypto/secp256k1/curve.go b/crypto/secp256k1/curve.go index fa1b199a3..a395e92fe 100644 --- a/crypto/secp256k1/curve.go +++ b/crypto/secp256k1/curve.go @@ -92,6 +92,12 @@ func (BitCurve *BitCurve) Params() *elliptic.CurveParams { // IsOnCurve returns true if the given (x,y) lies on the BitCurve. func (BitCurve *BitCurve) IsOnCurve(x, y *big.Int) bool { + if x.Sign() < 0 || x.Cmp(BitCurve.P) >= 0 { + return false + } + if y.Sign() < 0 || y.Cmp(BitCurve.P) >= 0 { + return false + } // y² = x³ + b y2 := new(big.Int).Mul(y, y) //y² y2.Mod(y2, BitCurve.P) //y²%P diff --git a/crypto/secp256k1/ext.h b/crypto/secp256k1/ext.h index e422fe4b4..38d245ebb 100644 --- a/crypto/secp256k1/ext.h +++ b/crypto/secp256k1/ext.h @@ -109,9 +109,13 @@ int secp256k1_ext_scalar_mul(const secp256k1_context* ctx, unsigned char *point, ARG_CHECK(scalar != NULL); (void)ctx; - secp256k1_fe_set_b32(&feX, point); - secp256k1_fe_set_b32(&feY, point+32); + if (!secp256k1_fe_set_b32(&feX, point) || !secp256k1_fe_set_b32(&feY, point+32)) { + return 0; + } secp256k1_ge_set_xy(&ge, &feX, &feY); + if (!secp256k1_ge_is_valid_var(&ge)) { + return 0; + } secp256k1_scalar_set_b32(&s, scalar, &overflow); if (overflow || secp256k1_scalar_is_zero(&s)) { ret = 0; diff --git a/crypto/secp256k1/secp256_test.go b/crypto/secp256k1/secp256_test.go index ef2a3a379..d3e4f4448 100644 --- a/crypto/secp256k1/secp256_test.go +++ b/crypto/secp256k1/secp256_test.go @@ -11,6 +11,7 @@ import ( "crypto/rand" "encoding/hex" "io" + "math/big" "testing" ) @@ -216,6 +217,41 @@ func TestRecoverSanity(t *testing.T) { } } +func TestIsOnCurveCoordinateRange(t *testing.T) { + curve := S256() + + // Valid point on curve: use generator + if !curve.IsOnCurve(curve.Gx, curve.Gy) { + t.Fatal("generator point should be on curve") + } + + // x = -1 should be rejected + if curve.IsOnCurve(big.NewInt(-1), curve.Gy) { + t.Fatal("negative x should not be on curve") + } + + // y = -1 should be rejected + if curve.IsOnCurve(curve.Gx, big.NewInt(-1)) { + t.Fatal("negative y should not be on curve") + } + + // x = P should be rejected + if curve.IsOnCurve(curve.P, curve.Gy) { + t.Fatal("x = P should not be on curve") + } + + // y = P should be rejected + if curve.IsOnCurve(curve.Gx, curve.P) { + t.Fatal("y = P should not be on curve") + } + + // x = P+7 could satisfy the curve equation mod P, but must be rejected + pPlus7 := new(big.Int).Add(curve.P, big.NewInt(7)) + if curve.IsOnCurve(pPlus7, curve.Gy) { + t.Fatal("x = P+7 should not be on curve") + } +} + func BenchmarkSign(b *testing.B) { _, seckey := generateKeyPair() msg := csprngEntropy(32) diff --git a/crypto/signature_nocgo.go b/crypto/signature_nocgo.go index 35fcb45f1..72b3b44ef 100644 --- a/crypto/signature_nocgo.go +++ b/crypto/signature_nocgo.go @@ -24,6 +24,7 @@ import ( "crypto/elliptic" "errors" "fmt" + "math/big" "github.com/btcsuite/btcd/btcec/v2" btc_ecdsa "github.com/btcsuite/btcd/btcec/v2/ecdsa" @@ -56,7 +57,11 @@ func SigToPub(hash, sig []byte) (*ecdsa.PublicKey, error) { if err != nil { return nil, err } - return pub.ToECDSA(), nil + return &ecdsa.PublicKey{ + Curve: S256(), + X: pub.X(), + Y: pub.Y(), + }, nil } // Sign calculates an ECDSA signature. @@ -71,7 +76,7 @@ func Sign(hash []byte, prv *ecdsa.PrivateKey) ([]byte, error) { if len(hash) != 32 { return nil, fmt.Errorf("hash is required to be exactly 32 bytes (%d)", len(hash)) } - if prv.Curve != btcec.S256() { + if prv.Curve != S256() { return nil, errors.New("private key curve is not secp256k1") } // ecdsa.PrivateKey -> btcec.PrivateKey @@ -126,7 +131,11 @@ func DecompressPubkey(pubkey []byte) (*ecdsa.PublicKey, error) { if err != nil { return nil, err } - return key.ToECDSA(), nil + return &ecdsa.PublicKey{ + Curve: S256(), + X: key.X(), + Y: key.Y(), + }, nil } // CompressPubkey encodes a public key to the 33-byte compressed format. The @@ -146,5 +155,22 @@ func CompressPubkey(pubkey *ecdsa.PublicKey) []byte { // S256 returns an instance of the secp256k1 curve. func S256() elliptic.Curve { - return btcec.S256() + return btCurve{btcec.S256()} +} + +type btCurve struct { + *btcec.KoblitzCurve +} + +func (curve btCurve) IsOnCurve(x, y *big.Int) bool { + if x == nil || y == nil { + return false + } + if x.Sign() < 0 || x.Cmp(btcec.Params().P) >= 0 { + return false + } + if y.Sign() < 0 || y.Cmp(btcec.Params().P) >= 0 { + return false + } + return curve.KoblitzCurve.IsOnCurve(x, y) } diff --git a/eth/protocols/snap/sync.go b/eth/protocols/snap/sync.go index adde1f67b..fed344d49 100644 --- a/eth/protocols/snap/sync.go +++ b/eth/protocols/snap/sync.go @@ -1568,6 +1568,9 @@ func (s *Syncer) revertAccountRequest(req *accountRequest) { // Remove the request from the tracked set s.lock.Lock() delete(s.accountReqs, req.id) + if _, ok := s.peers[req.peer]; ok { + s.accountIdlers[req.peer] = struct{}{} + } s.lock.Unlock() // If there's a timeout timer still running, abort it and mark the account @@ -1609,6 +1612,9 @@ func (s *Syncer) revertBytecodeRequest(req *bytecodeRequest) { // Remove the request from the tracked set s.lock.Lock() delete(s.bytecodeReqs, req.id) + if _, ok := s.peers[req.peer]; ok { + s.bytecodeIdlers[req.peer] = struct{}{} + } s.lock.Unlock() // If there's a timeout timer still running, abort it and mark the code @@ -1650,6 +1656,9 @@ func (s *Syncer) revertStorageRequest(req *storageRequest) { // Remove the request from the tracked set s.lock.Lock() delete(s.storageReqs, req.id) + if _, ok := s.peers[req.peer]; ok { + s.storageIdlers[req.peer] = struct{}{} + } s.lock.Unlock() // If there's a timeout timer still running, abort it and mark the storage @@ -1695,6 +1704,9 @@ func (s *Syncer) revertTrienodeHealRequest(req *trienodeHealRequest) { // Remove the request from the tracked set s.lock.Lock() delete(s.trienodeHealReqs, req.id) + if _, ok := s.peers[req.peer]; ok { + s.trienodeHealIdlers[req.peer] = struct{}{} + } s.lock.Unlock() // If there's a timeout timer still running, abort it and mark the trie node @@ -1736,6 +1748,9 @@ func (s *Syncer) revertBytecodeHealRequest(req *bytecodeHealRequest) { // Remove the request from the tracked set s.lock.Lock() delete(s.bytecodeHealReqs, req.id) + if _, ok := s.peers[req.peer]; ok { + s.bytecodeHealIdlers[req.peer] = struct{}{} + } s.lock.Unlock() // If there's a timeout timer still running, abort it and mark the code diff --git a/eth/protocols/snap/sync_test.go b/eth/protocols/snap/sync_test.go index 5fd750d95..c00a930ee 100644 --- a/eth/protocols/snap/sync_test.go +++ b/eth/protocols/snap/sync_test.go @@ -628,6 +628,215 @@ func setupSyncer(peers ...*testPeer) *Syncer { return syncer } +type revertRequestScenario struct { + name string + prepare func(s *Syncer, peerID string) + revert func(s *Syncer) + requestTracked func(s *Syncer) bool + peerIdle func(s *Syncer, peerID string) bool +} + +func makeRevertRequestScenarios() []func() revertRequestScenario { + return []func() revertRequestScenario{ + func() revertRequestScenario { + var req *accountRequest + return revertRequestScenario{ + name: "account", + prepare: func(s *Syncer, peerID string) { + task := &accountTask{} + req = &accountRequest{ + peer: peerID, + id: 1, + timeout: time.NewTimer(time.Hour), + stale: make(chan struct{}), + task: task, + } + task.req = req + s.accountReqs[req.id] = req + delete(s.accountIdlers, peerID) + }, + revert: func(s *Syncer) { s.revertAccountRequest(req) }, + requestTracked: func(s *Syncer) bool { + _, ok := s.accountReqs[1] + return ok + }, + peerIdle: func(s *Syncer, peerID string) bool { + _, ok := s.accountIdlers[peerID] + return ok + }, + } + }, + func() revertRequestScenario { + var req *bytecodeRequest + return revertRequestScenario{ + name: "bytecode", + prepare: func(s *Syncer, peerID string) { + task := &accountTask{codeTasks: make(map[common.Hash]struct{})} + req = &bytecodeRequest{ + peer: peerID, + id: 1, + timeout: time.NewTimer(time.Hour), + stale: make(chan struct{}), + hashes: []common.Hash{common.HexToHash("0x01")}, + task: task, + } + s.bytecodeReqs[req.id] = req + delete(s.bytecodeIdlers, peerID) + }, + revert: func(s *Syncer) { s.revertBytecodeRequest(req) }, + requestTracked: func(s *Syncer) bool { + _, ok := s.bytecodeReqs[1] + return ok + }, + peerIdle: func(s *Syncer, peerID string) bool { + _, ok := s.bytecodeIdlers[peerID] + return ok + }, + } + }, + func() revertRequestScenario { + var req *storageRequest + return revertRequestScenario{ + name: "storage", + prepare: func(s *Syncer, peerID string) { + task := &storageTask{} + req = &storageRequest{ + peer: peerID, + id: 1, + timeout: time.NewTimer(time.Hour), + stale: make(chan struct{}), + subTask: task, + } + task.req = req + s.storageReqs[req.id] = req + delete(s.storageIdlers, peerID) + }, + revert: func(s *Syncer) { s.revertStorageRequest(req) }, + requestTracked: func(s *Syncer) bool { + _, ok := s.storageReqs[1] + return ok + }, + peerIdle: func(s *Syncer, peerID string) bool { + _, ok := s.storageIdlers[peerID] + return ok + }, + } + }, + func() revertRequestScenario { + var req *trienodeHealRequest + return revertRequestScenario{ + name: "trienode heal", + prepare: func(s *Syncer, peerID string) { + task := &healTask{trieTasks: make(map[common.Hash]trie.SyncPath)} + req = &trienodeHealRequest{ + peer: peerID, + id: 1, + timeout: time.NewTimer(time.Hour), + stale: make(chan struct{}), + hashes: []common.Hash{common.HexToHash("0x02")}, + paths: []trie.SyncPath{nil}, + task: task, + } + s.trienodeHealReqs[req.id] = req + delete(s.trienodeHealIdlers, peerID) + }, + revert: func(s *Syncer) { s.revertTrienodeHealRequest(req) }, + requestTracked: func(s *Syncer) bool { + _, ok := s.trienodeHealReqs[1] + return ok + }, + peerIdle: func(s *Syncer, peerID string) bool { + _, ok := s.trienodeHealIdlers[peerID] + return ok + }, + } + }, + func() revertRequestScenario { + var req *bytecodeHealRequest + return revertRequestScenario{ + name: "bytecode heal", + prepare: func(s *Syncer, peerID string) { + task := &healTask{codeTasks: make(map[common.Hash]struct{})} + req = &bytecodeHealRequest{ + peer: peerID, + id: 1, + timeout: time.NewTimer(time.Hour), + stale: make(chan struct{}), + hashes: []common.Hash{common.HexToHash("0x03")}, + task: task, + } + s.bytecodeHealReqs[req.id] = req + delete(s.bytecodeHealIdlers, peerID) + }, + revert: func(s *Syncer) { s.revertBytecodeHealRequest(req) }, + requestTracked: func(s *Syncer) bool { + _, ok := s.bytecodeHealReqs[1] + return ok + }, + peerIdle: func(s *Syncer, peerID string) bool { + _, ok := s.bytecodeHealIdlers[peerID] + return ok + }, + } + }, + } +} + +func TestRevertRequestsReturnPeersToIdlePools(t *testing.T) { + t.Parallel() + + for _, makeScenario := range makeRevertRequestScenarios() { + scenario := makeScenario() + t.Run(scenario.name, func(t *testing.T) { + peer := newTestPeer("peer", t, func() {}) + syncer := setupSyncer(peer) + + scenario.prepare(syncer, peer.ID()) + if !scenario.requestTracked(syncer) { + t.Fatal("request should be tracked before revert") + } + if scenario.peerIdle(syncer, peer.ID()) { + t.Fatal("peer should be busy before revert") + } + + scenario.revert(syncer) + + if scenario.requestTracked(syncer) { + t.Fatal("request should be removed after revert") + } + if !scenario.peerIdle(syncer, peer.ID()) { + t.Fatal("peer should be returned to the idle pool") + } + }) + } +} + +func TestRevertRequestsSkipDroppedPeers(t *testing.T) { + t.Parallel() + + for _, makeScenario := range makeRevertRequestScenarios() { + scenario := makeScenario() + t.Run(scenario.name, func(t *testing.T) { + peer := newTestPeer("peer", t, func() {}) + syncer := setupSyncer(peer) + + scenario.prepare(syncer, peer.ID()) + if err := syncer.Unregister(peer.ID()); err != nil { + t.Fatalf("failed to unregister peer: %v", err) + } + + scenario.revert(syncer) + + if scenario.requestTracked(syncer) { + t.Fatal("request should be removed after revert") + } + if scenario.peerIdle(syncer, peer.ID()) { + t.Fatal("dropped peer should not be returned to the idle pool") + } + }) + } +} + // TestSync tests a basic sync with one peer func TestSync(t *testing.T) { t.Parallel() @@ -1694,7 +1903,7 @@ func TestSyncAccountPerformance(t *testing.T) { // Doing so would bring this number down to zero in this artificial testcase, // but only add extra IO for no reason in practice. if have, want := src.nTrienodeRequests, 1; have != want { - fmt.Printf(src.Stats()) + fmt.Print(src.Stats()) t.Errorf("trie node heal requests wrong, want %d, have %d", want, have) } } diff --git a/internal/ethapi/api_morph_test.go b/internal/ethapi/api_morph_test.go index e2c5c0e98..971f85471 100644 --- a/internal/ethapi/api_morph_test.go +++ b/internal/ethapi/api_morph_test.go @@ -8,8 +8,13 @@ import ( "github.com/morph-l2/go-ethereum/common" "github.com/morph-l2/go-ethereum/common/hexutil" + "github.com/morph-l2/go-ethereum/consensus" + "github.com/morph-l2/go-ethereum/core" "github.com/morph-l2/go-ethereum/core/rawdb" + "github.com/morph-l2/go-ethereum/core/state" + "github.com/morph-l2/go-ethereum/core/tracing" "github.com/morph-l2/go-ethereum/core/types" + "github.com/morph-l2/go-ethereum/core/vm" "github.com/morph-l2/go-ethereum/crypto" "github.com/morph-l2/go-ethereum/ethdb" "github.com/morph-l2/go-ethereum/params" @@ -155,7 +160,7 @@ type mockSetDefaultsBackend struct { header *types.Header } -func (m *mockSetDefaultsBackend) CurrentHeader() *types.Header { return m.header } +func (m *mockSetDefaultsBackend) CurrentHeader() *types.Header { return m.header } func (m *mockSetDefaultsBackend) ChainConfig() *params.ChainConfig { return m.chainConfig } func uint16VersionPtr(v uint8) *hexutil.Uint16 { @@ -163,6 +168,133 @@ func uint16VersionPtr(v uint8) *hexutil.Uint16 { return &h } +type estimateGasChainContext struct{} + +func (estimateGasChainContext) Engine() consensus.Engine { return nil } + +func (estimateGasChainContext) GetHeader(common.Hash, uint64) *types.Header { return nil } + +// estimateGasBackend is a minimal Backend that can run setDefaults -> DoEstimateGas +// against an in-memory state without spinning up a full blockchain. +type estimateGasBackend struct { + Backend + chainConfig *params.ChainConfig + header *types.Header + block *types.Block + state *state.StateDB + gasCap uint64 +} + +func newEstimateGasBackend(t *testing.T, sender common.Address) *estimateGasBackend { + t.Helper() + + db := rawdb.NewMemoryDatabase() + statedb, err := state.New(common.Hash{}, state.NewDatabase(db), nil) + if err != nil { + t.Fatalf("failed to create state db: %v", err) + } + statedb.SetBalance(sender, new(big.Int).Exp(big.NewInt(10), big.NewInt(18), nil), tracing.BalanceChangeUnspecified) + + header := &types.Header{ + Number: big.NewInt(1), + Time: 0, + GasLimit: 30_000_000, + BaseFee: big.NewInt(1), + Difficulty: big.NewInt(0), + } + return &estimateGasBackend{ + chainConfig: params.TestNoL1DataFeeChainConfig, + header: header, + block: types.NewBlockWithHeader(header), + state: statedb, + gasCap: header.GasLimit, + } +} + +func (m *estimateGasBackend) CurrentHeader() *types.Header { return types.CopyHeader(m.header) } + +func (m *estimateGasBackend) ChainConfig() *params.ChainConfig { return m.chainConfig } + +func (m *estimateGasBackend) RPCGasCap() uint64 { return m.gasCap } + +func (m *estimateGasBackend) BlockByNumberOrHash(context.Context, rpc.BlockNumberOrHash) (*types.Block, error) { + return m.block, nil +} + +func (m *estimateGasBackend) StateAndHeaderByNumberOrHash(context.Context, rpc.BlockNumberOrHash) (*state.StateDB, *types.Header, error) { + return m.state.Copy(), types.CopyHeader(m.header), nil +} + +func (m *estimateGasBackend) GetEVM(ctx context.Context, msg core.Message, statedb *state.StateDB, header *types.Header, vmConfig *vm.Config) (*vm.EVM, func() error, error) { + author := common.Address{} + blockCtx := core.NewEVMBlockContext(header, estimateGasChainContext{}, m.chainConfig, &author) + txCtx := core.NewEVMTxContext(msg) + return vm.NewEVM(blockCtx, txCtx, statedb, m.chainConfig, *vmConfig), func() error { return nil }, nil +} + +func makeAuthorizationList(count int) []types.SetCodeAuthorization { + auths := make([]types.SetCodeAuthorization, count) + for i := range auths { + auths[i] = types.SetCodeAuthorization{ + Address: common.Address{byte(i + 1)}, + Nonce: uint64(i), + } + } + return auths +} + +func TestSetDefaultsEstimateGasIncludesAuthorizationList(t *testing.T) { + sender := common.HexToAddress("0x1000000000000000000000000000000000000001") + to := common.HexToAddress("0x2000000000000000000000000000000000000002") + backend := newEstimateGasBackend(t, sender) + + nonce := hexutil.Uint64(0) + maxFee := (*hexutil.Big)(big.NewInt(10)) + tip := (*hexutil.Big)(big.NewInt(1)) + + tests := []struct { + name string + auths []types.SetCodeAuthorization + want uint64 + }{ + { + name: "legacy call without authorization list", + want: params.TxGas, + }, + { + name: "set code with one authorization", + auths: makeAuthorizationList(1), + want: params.TxGas + params.CallNewAccountGas, + }, + { + name: "set code with two authorizations", + auths: makeAuthorizationList(2), + want: params.TxGas + 2*params.CallNewAccountGas, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + args := TransactionArgs{ + From: &sender, + To: &to, + Nonce: &nonce, + MaxFeePerGas: maxFee, + MaxPriorityFeePerGas: tip, + AuthorizationList: tt.auths, + } + if err := args.setDefaults(context.Background(), backend); err != nil { + t.Fatalf("setDefaults failed: %v", err) + } + if args.Gas == nil { + t.Fatal("expected gas to be populated") + } + if have := uint64(*args.Gas); have != tt.want { + t.Fatalf("estimated gas mismatch: have %d want %d", have, tt.want) + } + }) + } +} + // TestSetDefaults_MorphTxVersionHeuristic tests the heuristic version defaulting logic // in TransactionArgs.setDefaults(): // - Version == nil + no V1 fields → V0 @@ -303,9 +435,9 @@ func TestSetDefaults_MorphTxVersionHeuristic(t *testing.T) { wantVersion: uint16Ref(types.MorphTxVersion1), }, { - name: "jade fork: no MorphTx fields → not MorphTx (version nil)", - headTime: 1000, - modify: func(args *TransactionArgs) {}, + name: "jade fork: no MorphTx fields → not MorphTx (version nil)", + headTime: 1000, + modify: func(args *TransactionArgs) {}, wantVersion: nil, }, @@ -392,16 +524,16 @@ func TestSetDefaults_MorphTxVersionHeuristic(t *testing.T) { wantErr: true, }, { - name: "jade fork: explicit V1 + FeeTokenID=0 + FeeLimit=nil → ok", - headTime: 1000, + name: "jade fork: explicit V1 + FeeTokenID=0 + FeeLimit=nil → ok", + headTime: 1000, modify: func(args *TransactionArgs) { args.Version = uint16VersionPtr(types.MorphTxVersion1) }, wantVersion: uint16Ref(types.MorphTxVersion1), }, { - name: "jade fork: explicit V1 + FeeTokenID=0 + FeeLimit=0 → ok", - headTime: 1000, + name: "jade fork: explicit V1 + FeeTokenID=0 + FeeLimit=0 → ok", + headTime: 1000, modify: func(args *TransactionArgs) { fid := hexutil.Uint16(0) args.FeeTokenID = &fid diff --git a/internal/ethapi/transaction_args.go b/internal/ethapi/transaction_args.go index 30c5cb2c4..977f2212a 100644 --- a/internal/ethapi/transaction_args.go +++ b/internal/ethapi/transaction_args.go @@ -269,6 +269,7 @@ func (args *TransactionArgs) setDefaults(ctx context.Context, b Backend) error { Value: args.Value, Data: (*hexutil.Bytes)(&data), AccessList: args.AccessList, + AuthorizationList: args.AuthorizationList, } pendingBlockNr := rpc.BlockNumberOrHashWithNumber(rpc.PendingBlockNumber) estimated, err := DoEstimateGas(ctx, b, callArgs, pendingBlockNr, b.RPCGasCap()) diff --git a/p2p/rlpx/rlpx_oracle_poc_test.go b/p2p/rlpx/rlpx_oracle_poc_test.go new file mode 100644 index 000000000..156825499 --- /dev/null +++ b/p2p/rlpx/rlpx_oracle_poc_test.go @@ -0,0 +1,57 @@ +package rlpx + +import ( + "bytes" + "errors" + "testing" + + "github.com/morph-l2/go-ethereum/crypto" + "github.com/morph-l2/go-ethereum/crypto/ecies" +) + +func TestHandshakeECIESInvalidCurveOracle(t *testing.T) { + initKey, err := crypto.GenerateKey() + if err != nil { + t.Fatal(err) + } + respKey, err := crypto.GenerateKey() + if err != nil { + t.Fatal(err) + } + + init := handshakeState{ + initiator: true, + remote: ecies.ImportECDSAPublic(&respKey.PublicKey), + } + authMsg, err := init.makeAuthMsg(initKey) + if err != nil { + t.Fatal(err) + } + packet, err := init.sealEIP8(authMsg) + if err != nil { + t.Fatal(err) + } + + var recv handshakeState + if _, err := recv.readMsg(new(authMsgV4), respKey, bytes.NewReader(packet)); err != nil { + t.Fatalf("expected valid packet to decrypt: %v", err) + } + + tampered := append([]byte(nil), packet...) + if len(tampered) < 2+65 { + t.Fatalf("unexpected packet length %d", len(tampered)) + } + tampered[2] = 0x04 + for i := 1; i < 65; i++ { + tampered[2+i] = 0x00 + } + + var recv2 handshakeState + _, err = recv2.readMsg(new(authMsgV4), respKey, bytes.NewReader(tampered)) + if err == nil { + t.Fatal("expected decryption failure for invalid curve point") + } + if !errors.Is(err, ecies.ErrInvalidPublicKey) { + t.Fatalf("unexpected error: %v", err) + } +}