NO TIMELOCK ON setProtocolFee() CAN LEAD TO SELLERS LOSING THEIR NFTs
In InfinityExchange.sol, there is no timelock on setProtocolFee(). This is the fee that is applied in orders, and determines how much the Exchange receives in fee VS how much the seller receives. But:
there is no maximum value limiting what fee can be set to
there is no timelock, ie PROTOCOL_FEE_BPS is set to its new value when setProtocolFee() is created.
Users will be incited to use the exchange if the fee is low (PROTOCOL_FEE_BPS is initially 2.5%). A malicious owner could effectively wait for enough activity to happen in the exchange, then set a very high fee, which would result in all further orders going through the exchange transferring the sales revenue from the buyers to the Exchange, resulting in the sellers effectively losing their NFTs.
Impact
Medium
Proof Of Concept
The malicious owner calls setProtocolFee(10000), setting PROTOCOL_FEE_BPS as 100%.
A user calls matchOneToOneOrders() for a specific NFT criteria.
A match is found and the order is executed automatically.
In the internal call to _execMatchOneToOneOrders(), protocolFee = (protocolFeeBps * execPrice) / 10000 = execPrice
remainingAmount = execPrice - protocolFee = 0
the contract transfers nothing to the seller, and execPrice to the exchange. The seller has effectively lost their NFT.
Tools Used
Manual Analysis
Recommended Mitigation Steps
Two things can be done:
set a maximum possible fee rate in setProtocolFee()
Lines of code
https://github.com/code-423n4/2022-06-infinity/blob/765376fa238bbccd8b1e2e12897c91098c7e5ac6/contracts/core/InfinityExchange.sol#L1267 https://github.com/code-423n4/2022-06-infinity/blob/765376fa238bbccd8b1e2e12897c91098c7e5ac6/contracts/core/InfinityExchange.sol#L725-L726 https://github.com/code-423n4/2022-06-infinity/blob/765376fa238bbccd8b1e2e12897c91098c7e5ac6/contracts/core/InfinityExchange.sol#L729
Vulnerability details
NO TIMELOCK ON
setProtocolFee()
CAN LEAD TO SELLERS LOSING THEIR NFTsIn
InfinityExchange.sol
, there is no timelock onsetProtocolFee()
. This is the fee that is applied in orders, and determines how much the Exchange receives in fee VS how much the seller receives. But:PROTOCOL_FEE_BPS
is set to its new value whensetProtocolFee()
is created.Users will be incited to use the exchange if the fee is low (
PROTOCOL_FEE_BPS
is initially2.5%
). A malicious owner could effectively wait for enough activity to happen in the exchange, then set a very high fee, which would result in all further orders going through the exchange transferring the sales revenue from the buyers to the Exchange, resulting in the sellers effectively losing their NFTs.Impact
Medium
Proof Of Concept
The malicious owner calls
setProtocolFee(10000)
, settingPROTOCOL_FEE_BPS
as 100%.matchOneToOneOrders()
for a specific NFT criteria._execMatchOneToOneOrders()
,protocolFee = (protocolFeeBps * execPrice) / 10000 = execPrice
remainingAmount = execPrice - protocolFee = 0
execPrice
to the exchange. The seller has effectively lost their NFT.Tools Used
Manual Analysis
Recommended Mitigation Steps
Two things can be done:
setProtocolFee()
setProtocolFee()
.