-
-
Published
Linked with GitHub
# Bid Comparison Bug
Regarding [this comparison](https://github.com/flashbots/mev-boost/blob/3721723a1b0df281fd8cc5b5cf805a858fa9511f/server/service.go#L296-L299) in `mev-boost::service::handleGetHeader`:
```go
// Skip if not a higher value
if result.Data != nil && responsePayload.Data.Message.Value.Cmp(&result.Data.Message.Value) < 1 {
return
}
```
I found that the [`Cmp`](https://github.com/flashbots/go-boost-utils/blob/713016cc455e74c342186f9eb9a1b5775bc7829a/types/utils.go#L5-L9) function is a custom function that wraps `bytes.Compare`:
```go
// Cmp compares one U256Str to another and returns an integer indicating whether a > b.
// The result will be 0 if a == b, -1 if a < b, and +1 if a > b.
func (n *U256Str) Cmp(b *U256Str) int {
return bytes.Compare(n[:], b[:])
}
```
According to [Go Documentation](https://pkg.go.dev/bytes#Compare):
> Compare returns an integer comparing two byte slices **lexicographically**.
The problem is, I do not think a lexicographic comparison is appropriate here. See the following tests:
```go
smaller := U256Str(common.HexToHash("0100000000000000000000000000000000000000000000000000000000000000"))
bigger := U256Str(common.HexToHash("0001000000000000000000000000000000000000000000000000000000000000"))
require.Equal(t, "1", smaller.String())
require.Equal(t, "256", bigger.String())
require.Equal(t, 0, smaller.Cmp(&smaller))
require.Equal(t, 0, bigger.Cmp(&bigger))
require.Equal(t, -1, smaller.Cmp(&bigger))
require.Equal(t, 1, bigger.Cmp(&smaller))
```
You should expect all of these tests to pass, but the last two tests, which compare `smaller` and `bigger`, will fail.
```
$ make test
go test ./...
ok github.com/flashbots/go-boost-utils/bls (cached)
--- FAIL: TestU256Str (0.00s)
common_test.go:79:
Error Trace: common_test.go:79
Error: Not equal:
expected: -1
actual : 1
Test: TestU256Str
FAIL
FAIL github.com/flashbots/go-boost-utils/types 0.116s
FAIL
make: *** [test] Error 1
```
This is because, lexicographically, `01 00` is greater than `00 01`. In this situation, a malicious builder could submit a bid that is smaller in value, but lexigraphically larger to always win the bid.
Here is one more group of example tests:
```go
smaller := U256Str(common.HexToHash("2222000000000000000000000000000000000000000000000000000000000000"))
bigger := U256Str(common.HexToHash("1111110000000000000000000000000000000000000000000000000000000000"))
require.Equal(t, "8738", smaller.String())
require.Equal(t, "1118481", bigger.String())
require.Equal(t, 0, smaller.Cmp(&smaller))
require.Equal(t, 0, bigger.Cmp(&bigger))
require.Equal(t, -1, smaller.Cmp(&bigger))
require.Equal(t, 1, bigger.Cmp(&smaller))
```
The last two tests will fail here too.