Description

The main Voyage contract also exposes the methods of the Multicall contract via this chain:

• Voyage is an instance of Diamond (by inheritance)

• Diamond allows to delegatecall any registered facet

  • One of the facets is PaymentsFacet

• PaymentsFacet is multicall by inheritance

  • Multicall has a multicall method that performs an arbitrary amount of delegatecalls to address(this), with arbitrary calldata

Any function called by multicall must not trust msg.value, since Multicall allows to perform multiple calls in the same transaction, preserving the same msg.value.

A function trusting msg.value might assume that the contract has received msg.value ETH from the caller and can spend it exclusively, which is not true in case the function is called multiple times in the same transaction by leveraging multicall.

Multicall allows to call any method exposed by any Voyage facet, including LoanFacet::buyNow, which assumes that msg.value ETH were sent by the caller as down payment for the requested NFT.

Impact

The buyNow function assumes the caller has sent msg.value ETH as down payment for the NFT. Luckily, an attacker cannot exploit this flawed assumption and use funds from the protocol pools to buy NFTs at a reduced price, as the contract will not have enough ETH to buy the NFT, causing a revert.

Recommendations

Adopt an explicit allowlist to limit which functions can be invoked by Multicall and ensure msg.value is not used by any of these functions. The buyNow function is the only one using msg.value in the commit under review.

Remediation

Multicalls and self permit have been removed from the code base entirely, the issue has therefore been remediated.