Audit the version of mev-boost implementing the builder API v0.2.0

[come-maiz]

We want to do an initial audit once mev-boost implements the builder API v0.2.0.

[come-maiz]

The audit will start on June 20th. The auditor is @lotusbumi, an anonymous security researcher fully trusted by Flashbots, with extensive experience in blockchain protocols, mainly in Ethereum. The audit process will be transparent and the results will be published in this repository.

[metachris]

Findings and solutions:

Issue	Fix	Version	Notes
signing library skips membership check (go-boost-utils)	flashbots/go-boost-utils#21	v0.3.1
server-side request forgery with unchecked redirects	#205	v0.7.4
server timeouts and maxheaderbytes configuration allows denial of service	#206	v0.7.4
client timeout can be set to insecure values	#210	v0.7.5
JSON decoder allows extra information to be loaded in memory	#211	v0.7.5	json-decoding using DisallowUnknownFields
nice-to-have: improved documentation of BLS usage in go-boost-utils			fix: add docs to go-boost-utils

[EvanVanNessEth]

Has anyone else noted some discomfort that the only audit is being done by an anon? As far as I can tell (via the completely blank Github profile), there is absolutely no reputation at stake.

It feels somewhat uncomfortable that as a community we'll be asking stakers to run software that is only audited by an anon with nothing at stake who we can't evaluate

I realize i am slightly overstating the case, but if something went wrong post-Merge....the critics would go wild and it'd look bad ex-post.

[come-maiz]

What's at stake is Flashbots reputation, not the reputation of our anon auditor.

In particular, my reputation for having chosen this auditor. I can make this call confidently and the team trusts my decision because I have many years researching what makes good and secure free software. Also because they are not a new auditor, their past work is extensive and brilliant, and we are happy they want to work with us in their new identity.

But well, I think in here reputations are not really relevant also for two reasons. First, we let their work speak for them. On this initial audit, they worked closely with @metachris and we are satisfied with the scope, depth, and result of their review. After some formatting, we will share the report.

Second, because we know that one audit doesn't make secure software. We have more audits planned, with more eyes and more hands, for this and other parts of the stack, together with more test suites, more calls for the community to help us test and review, and incentivized testing programs.

We also know that none of this is enough for making bug-free software. So we have contingency plans and mitigations in the protocol and the tooling we are building around it.

All the process will be clear and transparent on this repo as we get to execute it. We welcome and celebrate anybody who wants to participate.

@EvanVanNessEth let me know if you are still uncomfortable, and I can disclose more details.

[EvanVanNessEth]

i'm not trying to make trouble, it's just that this is a de facto part of the Merge and feels relatively less scrutinized?

put differently, "i annoy because i care" :)

happy to talk privately as well.

[come-maiz]

I'm not annoyed. I appreciate you care, and you hold us accountable. I appreciate a lot when people take the time to come and participate in our repository.

I want to make sure the process is satisfactory for any critics. Please keep sharing any concerns. Here is good. Dm is good.

[lotusbumi]

Even though Flashbots reputation on choosing me is on the line here, mine is as well. This is my first audit as an anon but not my first one. Definitely this will not be my last one, and i plan on keep making contributions with this github handle. I really appreciate the opportunity that was given to me by Flashbots and @ElOpio . Hopefully once the report is public the work done will also make a case for myself and my professional work .

[metachris]

Audit report:

---

MEV-Boost Security Assessment

MEV-Boost Security assessment for the Flashbots Collective

System overview

MEV-Boost is a system created by the Flashbots Collective to allow the out-of-protocol Proposer/Builder separation on PoS Eth once the Merge takes place. By making use of MEV-Boost, solo validators are expected to be able to get a share of the MEV on Ethereum without relying on running MEV strategies by themselves, but rather, receive from different builders bids to include specific block contents on their proposing slot.

The system consists in different actors:

• Builders: Block builders receive from different searchers bundles that will be "glued" together to create blocks. After creating them, they send a block header together with a bid value to relayers so that these block headers are exposed to validators. Afterwards, if validators select their bid, builders will unblind the contents of the block so that validators can add them to the canonical chain.
• Relayers: Relayers are actors that save registered validators and receive builders bid to make them available to validators.
• Validator clients: Beacon nodes that run the consensus logic. They register themselves to relayers through MEV-Boost, ask for specific block headers of the slots they propose and sign the block header so that the builder can unblind the block contents to include these contents on the proposed block.
• MEV-Boost: MEV-Boost is a software that receives HTTP requests from the validator client, and send HTTP requests to relayers, handling communications and the logic by which the block header is selected from the different relayers. In the current system, MEV-Boost will always select the block header with the highest bid.
• Execution clients: PoW ETH nodes that run the execution logic.
  It is important to note that the specifications are still a work in progress and that both the builder and relay software are still under heavy development.

Trust Assumptions

Proof of Stake Ethereum roadmap presents a trustless proposer/builder separation protocol. In the meantime, the current system will depend on trust assumptions of the different actors:

• Validators/MEV-Boost assume:
  • Relayers will not send blocks that could make validators get slashed.
  • Relayers will not send blocks bigger than the gas limit used in validator registration.
  • Relayers will not use a false bid value.
• Relayers assume:
  • Validators will send periodically registration information.
  • Builders will be online to share the block contents with validators if auction is won.
  • Builders will not use a false bid value.
• Builders assume:
  • Relayers will not share their blocks with other relayers/builders.
  • Relayers will verify that validators commit to a specific blockhash.

---

Findings

Critical

None.

High

None.

Medium

Signing library skips membership check

Update: Fixed in PR21

In the go-boost-utils repository, the VerifySignature function of the bls.go file makes use of the Verify function of the upstream blst library but with the sigGroupCheck variable set to false.

The IETF BLS Signature specification points outs about the usage of this variable in Section 5.3:

Some existing implementations skip the signature_subgroup_check
   invocation in CoreVerify (Section 2.7), whose purpose is ensuring
   that the signature is an element of a prime-order subgroup.  This
   check is REQUIRED of conforming implementations, for two reasons.

   1.  For most pairing-friendly elliptic curves used in practice, the
       pairing operation e (Section 1.3) is undefined when its input
       points are not in the prime-order subgroups of E1 and E2.  The
       resulting behavior is unpredictable, and may enable forgeries.

   2.  Even if the pairing operation behaves properly on inputs that are
       outside the correct subgroups, skipping the subgroup check breaks
       the strong unforgeability property.

Consider ensuring that the Verify function is called with the sigGroupCheck variable set to true to enforce unforgeability of signatures.

Low

Server-Side request forgery via unchecked redirects

Update: Fixed in PR205

A server side request forgery attack is the ability to force a system to create a new HTTP request by using the victim transport layer.

A malicious relayer can take advantage of server redirects and trigger GET or HEAD requests with empty body to any path or POST requests to any path with the initial payload body. This vulnerability can only be exploited if in the local network where the MEV-Boost software runs there is a reachable software with an existing vulnerability that can be triggered with the types of requests that an attacker is allowed to send.

As in most infrastructures MEV-Boost will have access to execution and validator clients, an underlying vulnerability in these critical systems could be triggered even though these clients are not exposed publicly.

Consider modifying the http.Client.CheckRedirect policy to ensure that no redirects will be followed.

Server timeouts and MaxHeaderBytes configuration allows denial of service attacks

Update: Fixed in PR210

Given the current http.Server timeouts configuration and the default MaxHeaderBytes value in use, it is possible for a malicious individual to perform a denial of service attack through HTTP requests on MEV-Boost.

By leveraging requests that asks for a path comprised of 1Mb of characters, it is possible to consume high amount of resources of the server running MEV-Boost. With the default configuration, we can slow other user's requests to the same MEV-Boost instance up to 15 seconds by only sending 100 requests via 10 threads with an attacker client.

Consider modifying the ReadHeaderTimeout value or the MaxHeaderBytes value to ensure the quick return of malicious requests to quickly free resources. Otherwise, consider making these values configurable by a command line flag. Moreover, consider publishing documentation around how to set up a public facing MEV-Boost or strictly documenting that this system should be only reachable by trusted parties.

Notes

BLS library missing documentation

Update: Fixed in PR205

As explained in the IETF BLS Signature specification, BLS signature schemes can be basic, message augmentation or proof of possession. Each one of these schemes work differently and requires different behavior for the client making use of the bslt library.

The client built in the bls.go file of the go-boost-repository uses a domain tag separator value corresponding to the proof of possession scheme. However, the library never make use of the PopProve and PopVerify functions which are the ones used in the proof of possession scheme.

Moreover, the bls client library does not support aggregated signatures, as the verification is only done with one message per verification.

Consider clearly documenting the expected behavior of the bls client library, the deviations from the specification and the security properties of the used scheme.

Client timeout can be set to insecure values

Update: Fixed in PR210

Even though MEV-Boost has a command line flag to set the timeout settings for the http.Client and that the default value is a "safe" value of two seconds, it is possible that a user sets this value to 0, disabling the client timeout and opening the door to being attacked by a malicious relayer that stalls the communication.

This is specially important during handleGetHeader function execution as any of the relayers can block the execution until every single request to the different relayers return.

Given the proposer boost, it is recommended for validators to include the block before 4 seconds into the slot or they will be probably be reorganized out of the canonical chain.

Consider checking that the timeout cannot be set to 0. Otherwise, consider documenting this behavior so that solo validators can understand the implications of this choice.

JSON Decoder allows extra information to be loaded in memory

Update: Fixed in PR211

In the handleGetPayload and handleRegisterValidator functions of the service.go file, the request payloads are processed by a Decoder without making use of the DisallowUnknownFields function, which would allow the Decoder to return an error when the destination is a struct and the input contains object keys which do not match any non-ignored, exported fields in the destination.

The usage of DisallowUnknownFields is recommended to avoid loading to memory and consuming resources decoding an invalid input.

---

Appendix A: Specification improvements

Even though this security audit was based on the MEV-Boost software, some security considerations were discovered for either the builder or relay software. A list of these is created to serve as base for further refinements and improvements of the specification of these actors to avoid these issues to trickle down on the implementations:

Relayer SignedBlindedBeaconBlock validation

To be slashed, a validator would need to sign two competing blocks for the block of the slot they are in charge of producing.

As such, relayers need to validate the SignedBlindedBeaconBlock signed by validators to be of the correct Slot and ProposerIndex before showing the unblinded block to the validator. If this is not checked, validators will be able to access the block contents and use these transactions to craft a new block, where they extract the MEV for themselves.

Validator ExecutionPayloadHeader validation

Validators should propose their block as in the first 4 seconds of the slot. Failure in doing so will result in higher probabilities of the block being re-organized due to the new proposer boost mechanism and the incentive of MEV extractors to create reorgs so that they can steal MEV extraction opportunties.

The GasLimit becomes then an important value, given the fact that if this number is high enough to allow blocks that are too big for the validator hardware to timely process the block contents it will probably end up in a missing opportunity for creation of a block in the canonical chain.

As such, validators should verify that the GasLimit value of the ExecutionPayloadHeader is lower than the gas limit value they used to register themselves to the relay network, which needs to be set to a sensible value that allows a validator hardware to timely create and validate the block.

---

Known-Issues

• Validator privacy
• Unconditional payments
• Late block proposal
• Header and bid publicly accessible
• Transaction cancellation
• Relays gossiping payloads between each other & multiple relays with the same payload
• Clarify relationship b/t builder, relay and proposer
• Value can be spoofed by relays
• Not validating relayer key