068.md

Koolex

medium

Lack of chainId validation for negative value

Summary

Lack of chainId validation for negative value

Vulnerability Detail

According to https://pkg.go.dev/math/big#Int.FillBytes of big.Int and its FilledBytes() function.

FillBytes sets buf to the absolute value of x

This means if you pass a negative value of x, it will give the same sequence of bytes. However, SigningHash function doesn't validate the chain_id against negative values. A verifer can gossip out a block message with an invalid chain Id (e.g. a negative value of a valid chain Id).

func SigningHash(domain [32]byte, chainID *big.Int, payloadBytes []byte) (common.Hash, error) {
	var msgInput [32 + 32 + 32]byte
	// domain: first 32 bytes
	copy(msgInput[:32], domain[:])
	// chain_id: second 32 bytes
	if chainID.BitLen() > 256 {
		return common.Hash{}, errors.New("chain_id is too large")
	}
	chainID.FillBytes(msgInput[32:64])
	// payload_hash: third 32 bytes, hash of encoded payload
	copy(msgInput[32:], crypto.Keccak256(payloadBytes))

	return crypto.Keccak256Hash(msgInput[:]), nil
}

https://github.com/sherlock-audit/2023-01-optimism/blob/main/optimism/op-node/p2p/signer.go#L26-L39

Impact

Any block signed by the sequencer for a valid chain Id is valid for an invalid chain Id. For example, a malicious verfier can pick a message signed for a chain Id 100 and gossip it out for P2P with a chain Id -100.

Code Snippet

Please create a file gossip_negativechainid_test under op-node/p2p directory and add the following code:

package p2p

import (
	"testing"
	"math/big"

	"github.com/stretchr/testify/require"
	"github.com/ethereum/go-ethereum/crypto"
)


func TestChainIdMalleability(t *testing.T) {

	negativeChainID :=	big.NewInt(-100)
	chainID :=	big.NewInt(100)

	var negativeMsgInput [32]byte
	var msgInput [32]byte

	negativeChainID.FillBytes(negativeMsgInput[:])
	chainID.FillBytes(msgInput[:])

	require.Equal(t, crypto.Keccak256(negativeMsgInput[:]), crypto.Keccak256(msgInput[:]))
}

Run the test as follows:

go test -run=TestChainIdMalleability ./p2p -v

The test will pass as the two hashes match

=== RUN   TestChainIdMalleability
--- PASS: TestChainIdMalleability (0.00s)
PASS
ok  	github.com/ethereum-optimism/optimism/op-node/p2p	0.637s

Tool used

Manual Review

Recommendation

Add a check for chain Id lower than zero

For example

   if(chainID.Cmp(big.NewInt(0)) == -1){
   	return common.Hash{}, errors.New("invalid chain_id")
   }