Skip to content

Commit

Permalink
Added gas limit cap to txs in txpool (#13352)
Browse files Browse the repository at this point in the history
fixes a potential vulnerability
  • Loading branch information
Giulio2002 authored Jan 9, 2025
1 parent e00c814 commit 91bda3a
Show file tree
Hide file tree
Showing 3 changed files with 77 additions and 14 deletions.
8 changes: 8 additions & 0 deletions txnprovider/txpool/pool.go
Original file line number Diff line number Diff line change
Expand Up @@ -901,6 +901,7 @@ func (p *TxPool) validateTx(txn *TxnSlot, isLocal bool, stateCache kvcache.Cache
}
return txpoolcfg.UnderPriced
}

gas, reason := txpoolcfg.CalcIntrinsicGas(uint64(txn.DataLen), uint64(txn.DataNonZeroLen), uint64(authorizationLen), nil, txn.Creation, true, true, isShanghai)
if txn.Traced {
p.logger.Info(fmt.Sprintf("TX TRACING: validateTx intrinsic gas idHash=%x gas=%d", txn.IDHash, gas))
Expand All @@ -917,6 +918,13 @@ func (p *TxPool) validateTx(txn *TxnSlot, isLocal bool, stateCache kvcache.Cache
}
return txpoolcfg.IntrinsicGas
}
if txn.Gas > p.blockGasLimit.Load() {
if txn.Traced {
p.logger.Info(fmt.Sprintf("TX TRACING: validateTx txn.gas > block gas limit idHash=%x gas=%d, block gas limit=%d", txn.IDHash, txn.Gas, p.blockGasLimit.Load()))
}
return txpoolcfg.GasLimitTooHigh
}

if !isLocal && uint64(p.all.count(txn.SenderID)) > p.cfg.AccountSlots {
if txn.Traced {
p.logger.Info(fmt.Sprintf("TX TRACING: validateTx marked as spamming idHash=%x slots=%d, limit=%d", txn.IDHash, p.all.count(txn.SenderID), p.cfg.AccountSlots))
Expand Down
82 changes: 68 additions & 14 deletions txnprovider/txpool/pool_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -733,7 +733,7 @@ func TestShanghaiValidateTxn(t *testing.T) {
asrt.NoError(err)
view, err := cache.View(ctx, tx)
asrt.NoError(err)

pool.blockGasLimit.Store(30_000_000)
reason := pool.validateTx(txn, false, view)

if reason != test.expected {
Expand All @@ -743,6 +743,64 @@ func TestShanghaiValidateTxn(t *testing.T) {
}
}

func TestTooHighGasLimitTxnValidation(t *testing.T) {
assert, require := assert.New(t), require.New(t)
ch := make(chan Announcements, 100)
coreDB, _ := temporaltest.NewTestDB(t, datadir.New(t.TempDir()))
db := memdb.NewTestPoolDB(t)
ctx, cancel := context.WithCancel(context.Background())
t.Cleanup(cancel)
cfg := txpoolcfg.DefaultConfig
sendersCache := kvcache.New(kvcache.DefaultCoherentConfig)
pool, err := New(ctx, ch, db, coreDB, cfg, sendersCache, *u256.N1, nil, nil, nil, nil, fixedgas.DefaultMaxBlobsPerBlock, nil, nil, func() {}, nil, log.New(), WithFeeCalculator(nil))
assert.NoError(err)
require.True(pool != nil)
var stateVersionID uint64 = 0
pendingBaseFee := uint64(200000)
// start blocks from 0, set empty hash - then kvcache will also work on this
h1 := gointerfaces.ConvertHashToH256([32]byte{})
change := &remote.StateChangeBatch{
StateVersionId: stateVersionID,
PendingBlockBaseFee: pendingBaseFee,
BlockGasLimit: 1000000,
ChangeBatch: []*remote.StateChange{
{BlockHeight: 0, BlockHash: h1},
},
}
var addr [20]byte
addr[0] = 1
v := types2.EncodeAccountBytesV3(2, uint256.NewInt(1*common.Ether), make([]byte, 32), 1)
change.ChangeBatch[0].Changes = append(change.ChangeBatch[0].Changes, &remote.AccountChange{
Action: remote.Action_UPSERT,
Address: gointerfaces.ConvertAddressToH160(addr),
Data: v,
})

tx, err := db.BeginRw(ctx)
require.NoError(err)
defer tx.Rollback()

err = pool.OnNewBlock(ctx, change, TxnSlots{}, TxnSlots{}, TxnSlots{})
assert.NoError(err)

{
var txnSlots TxnSlots
txnSlot := &TxnSlot{
Tip: *uint256.NewInt(300000),
FeeCap: *uint256.NewInt(300000),
Gas: 1000001,
Nonce: 2,
}
txnSlot.IDHash[0] = 1
txnSlots.Append(txnSlot, addr[:], true)

reasons, err := pool.AddLocalTxns(ctx, txnSlots)
assert.NoError(err)
assert.Len(reasons, 1)
assert.Equal(reasons[0], txpoolcfg.GasLimitTooHigh)
}
}

func TestSetCodeTxnValidationWithLargeAuthorizationValues(t *testing.T) {
maxUint256 := new(uint256.Int).SetAllOne()
ctx, cancel := context.WithCancel(context.Background())
Expand All @@ -756,6 +814,7 @@ func TestSetCodeTxnValidationWithLargeAuthorizationValues(t *testing.T) {
pool, err := New(ctx, ch, nil, coreDB, cfg, cache, chainID, common.Big0 /* shanghaiTime */, nil, /* agraBlock */
common.Big0 /* cancunTime */, common.Big0 /* pragueTime */, fixedgas.DefaultMaxBlobsPerBlock, nil, nil, func() {}, nil, logger, WithFeeCalculator(nil))
assert.NoError(t, err)
pool.blockGasLimit.Store(30_000_000)
tx, err := coreDB.BeginRw(ctx)
defer tx.Rollback()
assert.NoError(t, err)
Expand Down Expand Up @@ -803,14 +862,15 @@ func TestBlobTxnReplacement(t *testing.T) {
sendersCache := kvcache.New(kvcache.DefaultCoherentConfig)
pool, err := New(ctx, ch, db, coreDB, cfg, sendersCache, *u256.N1, common.Big0, nil, common.Big0, nil, fixedgas.DefaultMaxBlobsPerBlock, nil, nil, func() {}, nil, log.New(), WithFeeCalculator(nil))
assert.NoError(err)

require.True(pool != nil)
var stateVersionID uint64 = 0

h1 := gointerfaces.ConvertHashToH256([32]byte{})
change := &remote.StateChangeBatch{
StateVersionId: stateVersionID,
PendingBlockBaseFee: 200_000,
BlockGasLimit: 1000000,
BlockGasLimit: math.MaxUint64,
PendingBlobFeePerGas: 100_000,
ChangeBatch: []*remote.StateChange{
{BlockHeight: 0, BlockHash: h1},
Expand Down Expand Up @@ -958,7 +1018,6 @@ func makeBlobTxn() TxnSlot {
blobTxn.BlobHashes = make([]common.Hash, 2)
blobTxn.BlobHashes[0] = common.Hash(kzg.KZGToVersionedHash(commitment0))
blobTxn.BlobHashes[1] = common.Hash(kzg.KZGToVersionedHash(commitment1))

blobTxn.Tip = *tip
blobTxn.FeeCap = *feeCap
blobTxn.BlobFeeCap = *blobFeeCap
Expand Down Expand Up @@ -1093,7 +1152,7 @@ func TestBlobSlots(t *testing.T) {
change := &remote.StateChangeBatch{
StateVersionId: stateVersionID,
PendingBlockBaseFee: 200_000,
BlockGasLimit: 1000000,
BlockGasLimit: math.MaxUint64,
PendingBlobFeePerGas: 100_000,
ChangeBatch: []*remote.StateChange{
{BlockHeight: 0, BlockHash: h1},
Expand Down Expand Up @@ -1201,25 +1260,20 @@ func TestGasLimitChanged(t *testing.T) {
reasons, err := pool.AddLocalTxns(ctx, txnSlots)
assert.NoError(err)
for _, reason := range reasons {
assert.Equal(txpoolcfg.Success, reason, reason.String())
assert.Equal(reason, txpoolcfg.GasLimitTooHigh)
}

mtx, ok := pool.byHash[string(txnSlot1.IDHash[:])]
assert.True(ok)
assert.Zero(mtx.subPool&NotTooMuchGas, "Should be insufficient block space for the tx")

change.ChangeBatch[0].Changes = nil
change.BlockGasLimit = 150_000
err = pool.OnNewBlock(ctx, change, TxnSlots{}, TxnSlots{}, TxnSlots{})
assert.NoError(err)

assert.NotZero(mtx.subPool&NotTooMuchGas, "Should now have block space for the tx")

change.BlockGasLimit = 50_000
err = pool.OnNewBlock(ctx, change, TxnSlots{}, TxnSlots{}, TxnSlots{})
reasons, err = pool.AddLocalTxns(ctx, txnSlots)
assert.NoError(err)

assert.Zero(mtx.subPool&NotTooMuchGas, "Should now have block space (again) for the tx")
for _, reason := range reasons {
assert.Equal(txpoolcfg.Success, reason, reason.String())
}
}

// sender - immutable structure which stores only nonce and balance of account
Expand Down
1 change: 1 addition & 0 deletions txnprovider/txpool/txpoolcfg/txpoolcfg.go
Original file line number Diff line number Diff line change
Expand Up @@ -120,6 +120,7 @@ const (
BlobTxReplace DiscardReason = 30 // Cannot replace type-3 blob txn with another type of txn
BlobPoolOverflow DiscardReason = 31 // The total number of blobs (through blob txns) in the pool has reached its limit
NoAuthorizations DiscardReason = 32 // EIP-7702 transactions with an empty authorization list are invalid
GasLimitTooHigh DiscardReason = 33 // Gas limit is too high
)

func (r DiscardReason) String() string {
Expand Down

0 comments on commit 91bda3a

Please sign in to comment.