op-node: Fix OPB-01 (#3360)

* op-node: Fix OPB-01

`(*ExectionPayload).UnmarshalSSZ()` fails to properly validate the `transactionsOffset` and `extraDataOffset` values, allowing a malicious actor to crash multiple op-nodes by gossiping a P2P message containing a specially crafted `SSZExecutionPayload`.

* lint

op-node/Makefile

@@ -23,6 +23,7 @@ lint:
 fuzz:
 	go test -run NOTAREALTEST -v -fuzztime 10s -fuzz FuzzExecutionPayloadUnmarshal ./eth
 	go test -run NOTAREALTEST -v -fuzztime 10s -fuzz FuzzExecutionPayloadMarshalUnmarshal ./eth
+	go test -run NOTAREALTEST -v -fuzztime 10s -fuzz FuzzOBP01 ./eth
 	go test -run NOTAREALTEST -v -fuzztime 10s -fuzz FuzzL1InfoRoundTrip ./rollup/derive
 	go test -run NOTAREALTEST -v -fuzztime 10s -fuzz FuzzL1InfoAgainstContract ./rollup/derive
 	go test -run NOTAREALTEST -v -fuzztime 10s -fuzz FuzzUnmarshallLogEvent ./rollup/derive

op-node/eth/ssz.go

@@ -2,6 +2,7 @@ package eth
 
 import (
 	"encoding/binary"
+	"errors"
 	"fmt"
 	"io"
 	"sync"
@@ -24,6 +25,8 @@ var payloadBufPool = sync.Pool{New: func() any {
 	return &x
 }}
 
+var ErrBadTransactionOffset = errors.New("transactions offset is smaller than extra data offset, aborting")
+
 func (payload *ExecutionPayload) SizeSSZ() (full uint32) {
 	full = executionPayloadFixedPart + uint32(len(payload.ExtraData))
 	// One offset to each transaction
@@ -166,6 +169,9 @@ func (payload *ExecutionPayload) UnmarshalSSZ(scope uint32, r io.Reader) error {
 	copy(payload.BlockHash[:], buf[offset:offset+32])
 	offset += 32
 	transactionsOffset := binary.LittleEndian.Uint32(buf[offset : offset+4])
+	if transactionsOffset < extraDataOffset {
+		return ErrBadTransactionOffset
+	}
 	offset += 4
 	if offset != executionPayloadFixedPart {
 		panic("fixed part size is inconsistent")

op-node/eth/ssz_test.go

@@ -5,6 +5,8 @@ import (
 	"encoding/binary"
 	"testing"
 
+	"github.com/stretchr/testify/require"
+
 	"github.com/ethereum/go-ethereum/common"
 	"github.com/google/go-cmp/cmp"
 )
@@ -78,3 +80,47 @@ func FuzzExecutionPayloadMarshalUnmarshal(f *testing.F) {
 		}
 	})
 }
+
+func FuzzOBP01(f *testing.F) {
+	payload := &ExecutionPayload{
+		ExtraData: make([]byte, 32),
+	}
+	var buf bytes.Buffer
+	_, err := payload.MarshalSSZ(&buf)
+	require.NoError(f, err)
+	data := buf.Bytes()
+
+	f.Fuzz(func(t *testing.T, edOffset uint32, txOffset uint32) {
+		clone := make([]byte, len(data))
+		copy(clone, data)
+
+		binary.LittleEndian.PutUint32(clone[436:440], edOffset)
+		binary.LittleEndian.PutUint32(clone[504:508], txOffset)
+
+		var unmarshalled ExecutionPayload
+		err = unmarshalled.UnmarshalSSZ(uint32(len(clone)), bytes.NewReader(clone))
+		if err == nil {
+			t.Fatalf("expected a failure, but didn't get one")
+		}
+	})
+}
+
+// TestOPB01 verifies that the SSZ unmarshaling code
+// properly checks for the transactionOffset being larger
+// than the extraDataOffset.
+func TestOPB01(t *testing.T) {
+	payload := &ExecutionPayload{
+		ExtraData: make([]byte, 32),
+	}
+	var buf bytes.Buffer
+	_, err := payload.MarshalSSZ(&buf)
+	require.NoError(t, err)
+	data := buf.Bytes()
+
+	// transactions offset is set between indices 504 and 508
+	copy(data[504:508], make([]byte, 4))
+
+	var unmarshalled ExecutionPayload
+	err = unmarshalled.UnmarshalSSZ(uint32(len(data)), bytes.NewReader(data))
+	require.Equal(t, ErrBadTransactionOffset, err)
+}