code-423n4 / 2021-09-defiprotocol-findings

1 stars 0 forks source link

Re-entrancy in `Factory.createBasket` #219

Open code423n4 opened 2 years ago

code423n4 commented 2 years ago

Handle

cmichel

Vulnerability details

A basket creator can specify a custom token that allows them to re-enter in Factory.createBasket.

Impact

As new auction and basket contracts are created every time, no cross-basket issues arise. However, note that the official BasketCreated event is emitted for all of them, but only the last basket is stored for the idNumber. This could lead to issues for some backend / frontend scripts that use the BasketCreated event.

Recommended Mitigation Steps

Set _proposals[idNumber].basket = address(newBasket); immediately after the newBasket contract clone has been created to avoid the re-entrancy.

GalloDaSballo commented 2 years ago

Given the fact that the warden hasn't identified any high or mid risk exploit via re-entrancy, I'll keep the finding as low severity. (Lack of poc)

However notice that re-entrancy could be used for high severity exploits, particular attention should be put for the functions that handle transfers before updating state, see: https://github.com/code-423n4/2021-09-defiProtocol/blob/52b74824c42acbcd64248f68c40128fe3a82caf6/contracts/contracts/Auction.sol#L140