Platform: Code4rena
Start Date: 11/11/2022
Pot Size: $36,500 USDC
Total HM: 5
Participants: 62
Period: 3 days
Judge: berndartmueller
Id: 181
League: ETH
Rank: 17/62
Findings: 2
Award: $572.42
🌟 Selected for report: 0
🚀 Solo Findings: 0
🌟 Selected for report: rotcivegaf
Also found by: 0x4non, 0xDecorativePineapple, 9svR6w, Trust, V_B, adriro, ajtra, aviggiano, brgltd, carlitox477, chaduke, codexploder, corerouter, joestakey, ladboy233, s3cunda, saian, wait
66.8068 USDC - $66.81
When a caller to Exchange's execute() or bulkExecute() includes more ETH than is required to complete the transaction(s), _returnDust is intended to return this excess back to the caller. However, _returnDust() does not validate that the call() it performs to return this ETH succeeds. Returning the ETH may fail silently for example when the caller is a contract not implementing a function to receive ETH, or reaching out of gas, etc. The excess ETH will remain in the Exchange contract and can be taken by any other caller to the Exchange who sends excess ETH, triggers _returnDust() again, and receives the ETH left in the Exchange by the earlier caller who failed to receive it.
The danger is in a caller to Exchange expecting that any excess ETH provided (perhaps an upper bound of ETH expected for an unfamiliar transaction policy) will be returned. When in fact, the excess may not be returned in some cases.
The call I referred to is here: https://github.com/code-423n4/2022-11-non-fungible/blob/323b7cbf607425dd81da96c0777c8b12e800305d/contracts/Exchange.sol#L216 This is the first code4rena contest I have participated in, and unfortunately I did not have time to create a demonstration Proof of Concept.
Validate the status returned by the call() in _returnDust()
#0 - c4-judge
2022-11-16T11:58:46Z
berndartmueller marked the issue as duplicate of #90
#1 - c4-judge
2022-11-16T11:58:50Z
berndartmueller marked the issue as satisfactory
#2 - c4-judge
2022-12-06T14:17:28Z
berndartmueller changed the severity to 2 (Med Risk)
🌟 Selected for report: Trust
Also found by: 0xSmartContract, 0xhacksmithh, 9svR6w, Josiah, deliriusz, ladboy233, zaskoh
505.6113 USDC - $505.61
The choice of policy to use for a transaction is determined by the listingTime. The listingTime can be supplied by the caller of execute()/bulkExecute() and can be arbitrary as along as it passes validation. And the policy of a given order is used to determine the transaction price when the order is matched with another order. The owner of the Exchange can change the policyManager, which determines which policies are whitelisted, at any time (or they can change which policies are whitelisted by the manager).
A malicious exchange operator could set a policy manager that allows arbitrary policies, then create a policy that allows payment at low prices, and finally supply a series of orders at low price, with this policy manager, and an early but valid listingTime set. This would allow purchasing offered tokens at low prices for example.
https://github.com/code-423n4/2022-11-non-fungible/blob/323b7cbf607425dd81da96c0777c8b12e800305d/contracts/Exchange.sol#L543 This is my first code4rena contest and unfortunately I did not have time to develop a poc.
Increase validation of listingTime and add a timelock when changing the policy manager or changing which policies are whitelisted by the policy manager.
#0 - c4-judge
2022-11-17T10:27:58Z
berndartmueller marked the issue as duplicate of #179
#1 - c4-judge
2022-11-17T10:28:02Z
berndartmueller marked the issue as satisfactory