Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Crash in GetHeaderV1 method call #100

Closed
0xpanoramix opened this issue Apr 23, 2022 · 1 comment
Closed

Crash in GetHeaderV1 method call #100

0xpanoramix opened this issue Apr 23, 2022 · 1 comment

Comments

@0xpanoramix
Copy link
Contributor

0xpanoramix commented Apr 23, 2022 •
edited
Loading

The GetHeaderV1 method is crashing when nil parameter is provided as the blockHash value

Description

I was reading the code and testing the project when I realized that the GetHeaderV1 method takes a pointer as parameter, the blockHash.

Here is the original function code :

// GetHeaderV1 TODO
func (m *BoostService) GetHeaderV1(ctx context.Context, blockHash *string) (*GetHeaderResponse, error) {
	method := "builder_getHeaderV1"
	logMethod := m.log.WithField("method", method)

	if len(*blockHash) != 66 {
		return nil, fmt.Errorf("invalid block hash: %s", *blockHash)
	}
        // The rest of the function
}

Dereferencing the pointer without pre-verification of its content (here a non-nil check) causes the method call to crash when providing a nil value for the blockHash parameter.

I know that the function is executed in a goroutine, but still, I think it impacts stability.

Your environment

  • OS and version: macOS Monterey v12.3.1
  • branch/commit hash that causes this issue: 1108378

Steps to reproduce

  • How to reproduce this issue ?

    As written in the project's README, I built mev-boost using the following command :
make build

And executed the binary using (don't mind the port, the default one was used by another program):

./mev-boost -port 8080

Then, in a new terminal, I ran the following curl command :

curl -X POST http://127.0.0.1:8080/ -H 'Content-Type: application/json' --data '{"jsonrpc":"2.0","method":"builder_getHeaderV1","params":[],"id":1}'

To confirm the behaviour, I also wrote the following test before patching the function :

func TestE2E_GetHeaderError(t *testing.T) {
	relay1 := setupMockRelay()
	server, err := newTestBoostRPCServer([]string{relay1.URL})
	require.Nil(t, err, err)
	defer server.Stop()

	client := gethRpc.DialInProc(server)
	defer client.Close()

	res := new(GetHeaderResponse)
	err = client.Call(&res, "builder_getHeaderV1", nil)
	require.Error(t, err)
}

And both of them lead to a goroutine crash.

  • Where the issue is ?

    As mentionned in the description, the issue is caused by dereferencing a nil pointer.

Expected behaviour

The server should reject my invalid request with an error message (I suppose ?).

Actual behaviour

The goroutine handling the request crashed. Check out the logs for more info.

Logs

With the CURL command :

INFO[0000] mev-boost dev                                 prefix=cmd/mev-boost
INFO[0000] listening on  localhost:8080                  prefix=cmd/mev-boost
ERROR[04-24|01:11:44.095] RPC method builder_getHeaderV1 crashed: runtime error: invalid memory address or nil pointer dereference
goroutine 40 [running]:
github.com/ethereum/go-ethereum/rpc.(*callback).call.func1()
        /Users/ptitluca/go/pkg/mod/github.com/!marius!van!der!wijden/go-ethereum@v1.8.22-0.20211208130742-dd90624af970/rpc/service.go:200 +0x74
panic({0x10123db80, 0x101540e20})
        /Users/ptitluca/go/go1.18.1/src/runtime/panic.go:838 +0x204
github.com/flashbots/mev-boost/lib.(*BoostService).GetHeaderV1(0x14000236180, {0x1012b5028?, 0x14000222f40}, 0x0)
        /Users/ptitluca/GolandProjects/mev-boost/lib/service.go:144 +0xe8
reflect.Value.call({0x14000236200?, 0x1400021e198?, 0x1017b8a68?}, {0x10112b5f4, 0x4}, {0x14000228960, 0x3, 0x1010fcc6c?})
        /Users/ptitluca/go/go1.18.1/src/reflect/value.go:556 +0x5e4
reflect.Value.Call({0x14000236200?, 0x1400021e198?, 0x14000235968?}, {0x14000228960, 0x3, 0x3})
        /Users/ptitluca/go/go1.18.1/src/reflect/value.go:339 +0x98
github.com/ethereum/go-ethereum/rpc.(*callback).call(0x1400020eae0, {0x1012b5028?, 0x14000222f40}, {0x14000232228, 0x13}, {0x14000230420, 0x1, 0x1010f7370?})
        /Users/ptitluca/go/pkg/mod/github.com/!marius!van!der!wijden/go-ethereum@v1.8.22-0.20211208130742-dd90624af970/rpc/service.go:206 +0x38c
github.com/ethereum/go-ethereum/rpc.(*handler).runMethod(0x14000235958?, {0x1012b5028?, 0x14000222f40?}, 0x14000293d50, 0x1?, {0x14000230420?, 0x100010000?, 0x0?})
        /Users/ptitluca/go/pkg/mod/github.com/!marius!van!der!wijden/go-ethereum@v1.8.22-0.20211208130742-dd90624af970/rpc/handler.go:389 +0x44
github.com/ethereum/go-ethereum/rpc.(*handler).handleCall(0x1400023c240, 0x1400027f2c0, 0x14000293d50)
        /Users/ptitluca/go/pkg/mod/github.com/!marius!van!der!wijden/go-ethereum@v1.8.22-0.20211208130742-dd90624af970/rpc/handler.go:337 +0x1dc
github.com/ethereum/go-ethereum/rpc.(*handler).handleCallMsg(0x1400023c240, 0x1400027f2c0?, 0x14000293d50)
        /Users/ptitluca/go/pkg/mod/github.com/!marius!van!der!wijden/go-ethereum@v1.8.22-0.20211208130742-dd90624af970/rpc/handler.go:298 +0x80
github.com/ethereum/go-ethereum/rpc.(*handler).handleMsg.func1(0x1400027f2c0)
        /Users/ptitluca/go/pkg/mod/github.com/!marius!van!der!wijden/go-ethereum@v1.8.22-0.20211208130742-dd90624af970/rpc/handler.go:139 +0x38
github.com/ethereum/go-ethereum/rpc.(*handler).startCallProc.func1()
        /Users/ptitluca/go/pkg/mod/github.com/!marius!van!der!wijden/go-ethereum@v1.8.22-0.20211208130742-dd90624af970/rpc/handler.go:226 +0xc4
created by github.com/ethereum/go-ethereum/rpc.(*handler).startCallProc
        /Users/ptitluca/go/pkg/mod/github.com/!marius!van!der!wijden/go-ethereum@v1.8.22-0.20211208130742-dd90624af970/rpc/handler.go:222 +0x90
 
WARN [04-24|01:11:44.097] Served builder_getHeaderV1               conn=127.0.0.1:50085 reqid=1 t=1.580458ms err="method handler crashed"

While running the test :

=== RUN   TestE2E_GetHeaderError
ERROR[04-24|01:13:57.100] RPC method builder_getHeaderV1 crashed: runtime error: invalid memory address or nil pointer dereference
goroutine 51 [running]:
github.com/ethereum/go-ethereum/rpc.(*callback).call.func1()
	/Users/ptitluca/go/pkg/mod/github.com/!marius!van!der!wijden/go-ethereum@v1.8.22-0.20211208130742-dd90624af970/rpc/service.go:200 +0x74
panic({0x1052defe0, 0x105640c00})
	/Users/ptitluca/go/go1.18.1/src/runtime/panic.go:838 +0x204
github.com/flashbots/mev-boost/lib.(*BoostService).GetHeaderV1(0x140000ac500, {0x1053691a0?, 0x14000312080}, 0x0)
	/Users/ptitluca/GolandProjects/mev-boost/lib/service.go:144 +0xe8
reflect.Value.call({0x140000ac580?, 0x140000a41f8?, 0x105990f18?}, {0x1051ae701, 0x4}, {0x1400031e050, 0x3, 0x105145f4c?})
	/Users/ptitluca/go/go1.18.1/src/reflect/value.go:556 +0x5e4
reflect.Value.Call({0x140000ac580?, 0x140000a41f8?, 0x140003242d8?}, {0x1400031e050, 0x3, 0x3})
	/Users/ptitluca/go/go1.18.1/src/reflect/value.go:339 +0x98
github.com/ethereum/go-ethereum/rpc.(*callback).call(0x1400009cd80, {0x1053691a0?, 0x14000312080}, {0x14000334000, 0x13}, {0x1400031a180, 0x1, 0x105140430?})
	/Users/ptitluca/go/pkg/mod/github.com/!marius!van!der!wijden/go-ethereum@v1.8.22-0.20211208130742-dd90624af970/rpc/service.go:206 +0x38c
github.com/ethereum/go-ethereum/rpc.(*handler).runMethod(0x140003242d0?, {0x1053691a0?, 0x14000312080?}, 0x14000332000, 0x1?, {0x1400031a180?, 0x60000101010000?, 0x12c844800?})
	/Users/ptitluca/go/pkg/mod/github.com/!marius!van!der!wijden/go-ethereum@v1.8.22-0.20211208130742-dd90624af970/rpc/handler.go:389 +0x44
github.com/ethereum/go-ethereum/rpc.(*handler).handleCall(0x140001402d0, 0x1400030e360, 0x14000332000)
	/Users/ptitluca/go/pkg/mod/github.com/!marius!van!der!wijden/go-ethereum@v1.8.22-0.20211208130742-dd90624af970/rpc/handler.go:337 +0x1dc
github.com/ethereum/go-ethereum/rpc.(*handler).handleCallMsg(0x140001402d0, 0x1400030e360?, 0x14000332000)
	/Users/ptitluca/go/pkg/mod/github.com/!marius!van!der!wijden/go-ethereum@v1.8.22-0.20211208130742-dd90624af970/rpc/handler.go:298 +0x80
github.com/ethereum/go-ethereum/rpc.(*handler).handleMsg.func1(0x1400030e360)
	/Users/ptitluca/go/pkg/mod/github.com/!marius!van!der!wijden/go-ethereum@v1.8.22-0.20211208130742-dd90624af970/rpc/handler.go:139 +0x38
github.com/ethereum/go-ethereum/rpc.(*handler).startCallProc.func1()
	/Users/ptitluca/go/pkg/mod/github.com/!marius!van!der!wijden/go-ethereum@v1.8.22-0.20211208130742-dd90624af970/rpc/handler.go:226 +0xc4
created by github.com/ethereum/go-ethereum/rpc.(*handler).startCallProc
	/Users/ptitluca/go/pkg/mod/github.com/!marius!van!der!wijden/go-ethereum@v1.8.22-0.20211208130742-dd90624af970/rpc/handler.go:222 +0x90

Proposed solution

A verification should be made before dereferencing the pointer to blockHash, and return an error if no such value has been provided.
I have created a PR for this in #101.
@0xpanoramix 0xpanoramix mentioned this issue Apr 23, 2022
Merged
@metachris
Copy link
Collaborator

Thanks, the fix is merged.

This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@metachris @0xpanoramix