Commit 2b55f5ee authored by Matthew Slipper's avatar Matthew Slipper Committed by GitHub

op-node: Fix OPB-04 (#3530)

* op-node: Fix OPB-04

The length of the ExtraData field was not being checked, which could lead to panics in the MarshalSSZ function.

Fixes ENG-2614

* code review fixes

* increase resource size
Co-authored-by: default avatarmergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
parent 3883f34b
...@@ -228,6 +228,7 @@ jobs: ...@@ -228,6 +228,7 @@ jobs:
bedrock-go-tests: bedrock-go-tests:
docker: docker:
- image: ethereumoptimism/ci-builder:latest - image: ethereumoptimism/ci-builder:latest
resource_class: xlarge
steps: steps:
- checkout - checkout
- run: - run:
......
...@@ -5,6 +5,7 @@ import ( ...@@ -5,6 +5,7 @@ import (
"errors" "errors"
"fmt" "fmt"
"io" "io"
"math"
"sync" "sync"
) )
...@@ -18,6 +19,10 @@ const executionPayloadFixedPart = 32 + 20 + 32 + 32 + 256 + 32 + 8 + 8 + 8 + 8 + ...@@ -18,6 +19,10 @@ const executionPayloadFixedPart = 32 + 20 + 32 + 32 + 256 + 32 + 8 + 8 + 8 + 8 +
// MAX_TRANSACTIONS_PER_PAYLOAD in consensus spec // MAX_TRANSACTIONS_PER_PAYLOAD in consensus spec
const maxTransactionsPerPayload = 1 << 20 const maxTransactionsPerPayload = 1 << 20
// ErrExtraDataTooLarge occurs when the ExecutionPayload's ExtraData field
// is too large to be properly represented in SSZ.
var ErrExtraDataTooLarge = errors.New("extra data too large")
// The payloads are small enough to read and write at once. // The payloads are small enough to read and write at once.
// But this happens often enough that we want to avoid re-allocating buffers for this. // But this happens often enough that we want to avoid re-allocating buffers for this.
var payloadBufPool = sync.Pool{New: func() any { var payloadBufPool = sync.Pool{New: func() any {
...@@ -57,6 +62,10 @@ func unmarshalBytes32LE(in []byte, z *Uint256Quantity) { ...@@ -57,6 +62,10 @@ func unmarshalBytes32LE(in []byte, z *Uint256Quantity) {
// MarshalSSZ encodes the ExecutionPayload as SSZ type // MarshalSSZ encodes the ExecutionPayload as SSZ type
func (payload *ExecutionPayload) MarshalSSZ(w io.Writer) (n int, err error) { func (payload *ExecutionPayload) MarshalSSZ(w io.Writer) (n int, err error) {
if len(payload.ExtraData) > math.MaxUint32-executionPayloadFixedPart {
return 0, ErrExtraDataTooLarge
}
scope := payload.SizeSSZ() scope := payload.SizeSSZ()
buf := *payloadBufPool.Get().(*[]byte) buf := *payloadBufPool.Get().(*[]byte)
......
...@@ -3,6 +3,7 @@ package eth ...@@ -3,6 +3,7 @@ package eth
import ( import (
"bytes" "bytes"
"encoding/binary" "encoding/binary"
"math"
"testing" "testing"
"github.com/google/go-cmp/cmp" "github.com/google/go-cmp/cmp"
...@@ -124,3 +125,25 @@ func TestOPB01(t *testing.T) { ...@@ -124,3 +125,25 @@ func TestOPB01(t *testing.T) {
err = unmarshalled.UnmarshalSSZ(uint32(len(data)), bytes.NewReader(data)) err = unmarshalled.UnmarshalSSZ(uint32(len(data)), bytes.NewReader(data))
require.Equal(t, ErrBadTransactionOffset, err) require.Equal(t, ErrBadTransactionOffset, err)
} }
// TestOPB04 verifies that the SSZ marshaling code
// properly returns an error when the ExtraData field
// cannot be represented in the outputted SSZ.
func TestOPB04(t *testing.T) {
// First, test the maximum len - which in this case is the max uint32
// minus the execution payload fixed part.
payload := &ExecutionPayload{
ExtraData: make([]byte, math.MaxUint32-executionPayloadFixedPart),
}
var buf bytes.Buffer
_, err := payload.MarshalSSZ(&buf)
require.NoError(t, err)
buf.Reset()
payload = &ExecutionPayload{
ExtraData: make([]byte, math.MaxUint32-executionPayloadFixedPart+1),
}
_, err = payload.MarshalSSZ(&buf)
require.Error(t, err)
require.Equal(t, ErrExtraDataTooLarge, err)
}
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment