search

code-423n4 / 2022-06-putty-findings

5 stars 0 forks source link

Long whitelist could cause out of gas error #137

Open code423n4 opened 2 years ago

code423n4 commented 2 years ago

Lines of code

https://github.com/code-423n4/2022-06-putty/blob/3b6b844bc39e897bd0bbb69897f2deff12dc3893/contracts/src/PuttyV2.sol#L284

Vulnerability details

Impact

Transaction reverts due to out of gas error because whitelist is too long

Proof of Concept

L670 simply loops through the entire list of whitelisted addresses until it matches msg.sender. If list is long and msg.sender is further down on the list, out of gas errors could easily happen. Additionally this could be use maliciously on counter offers to cause other user high gas fees.

Tools Used

Recommended Mitigation Steps

Require the length of the whitelist to be less than some max length (i.e. 64) when calling fillOrder()

require(order.whitelist.length < 64)

outdoteth commented 2 years ago

Any out of gas error here will show up in the user's wallet as an error. This will prevent them from wasting gas and submitting the tx to get a revert on chain. Technically this is a valid finding, but think it should be marked as low severity.

HickupHH3 commented 2 years ago

there's control over the whitelist off-chain. it can therefore be enforced to be of reasonable length or trimmed if necessary. non-crit IMO.

warden has no QA report, this issue will be the primary