Reentrancy in AuctionFacet.sol will manipulate order of emit
Summary
In AuctionFacet.sol I found a Reentrancy bug. which will result in changing the order of emit .
Vulnerability Detail
In AuctionFacet.sol if someone buys NFT then it calls safeTransferFrom() to transfers NFT to buyer . As we know that safeTransferFrom() will call onERC721Received() before transferring NFT to the buyer. if buyer is malicious . lets say buyer is old owner of nft who borrowed token by giving collateral as this NFT . then he can write exploited code inside onERC721Received() to call back to claimAsBorrower() to claim his shares. but in emit log to will shows that before selling his NFT in action he claimed his share price .
POC :
1) Let's say bob has borrowed some tokens using his NFT. If he is unable to repay in the required time then , his NFT will come to auction .
2) now in action bob will buy this NFT by calling buy() . while sending NFT to bob it will call safeTransferFrom().
3) safeTransferFrom() will trigger onERC721Received() in bobs contract in which he will call claimAsBorrower() to claim his share price . while claiming his share claimAsBorrower() will emit Claim event
4) After this it will call remaining emit Buy event . In logs it shows that bob has claimed his share price before selling his NFT in auction.
Impact
According to functionality code should emit Buy event then a Claim event . but in this case malicious user is able to manipulate this emit events . emit logs shows that before selling NFT user claimed his share price .
SPYBOY
medium
Reentrancy in AuctionFacet.sol will manipulate order of emit
Summary
In AuctionFacet.sol I found a Reentrancy bug. which will result in changing the order of emit .
Vulnerability Detail
In AuctionFacet.sol if someone buys NFT then it calls
safeTransferFrom()to transfers NFT to buyer . As we know thatsafeTransferFrom()will call onERC721Received() before transferring NFT to the buyer. if buyer is malicious . lets say buyer is old owner of nft who borrowed token by giving collateral as this NFT . then he can write exploited code inside onERC721Received() to call back toclaimAsBorrower()to claim his shares. but in emit log to will shows that before selling his NFT in action he claimed his share price .POC : 1) Let's say bob has borrowed some tokens using his NFT. If he is unable to repay in the required time then , his NFT will come to auction . 2) now in action bob will buy this NFT by calling
buy(). while sending NFT to bob it will callsafeTransferFrom(). 3)safeTransferFrom()will triggeronERC721Received()in bobs contract in which he will callclaimAsBorrower()to claim his share price . while claiming his shareclaimAsBorrower()will emitClaimevent 4) After this it will call remaining emitBuyevent . In logs it shows that bob has claimed his share price before selling his NFT in auction.Impact
According to functionality code should emit
Buyevent then aClaimevent . but in this case malicious user is able to manipulate this emit events . emit logs shows that before selling NFT user claimed his share price .Code Snippet
buy in auction : https://github.com/kairos-loan/kairos-contracts/blob/ce49230ab5255662d287c4944229cf411725de3f/src/AuctionFacet.sol#L25-L29 claimAsBorrower() : https://github.com/kairos-loan/kairos-contracts/blob/ce49230ab5255662d287c4944229cf411725de3f/src/ClaimFacet.sol#L58-L88 safeTransferFrom in auction : https://github.com/kairos-loan/kairos-contracts/blob/ce49230ab5255662d287c4944229cf411725de3f/src/AuctionFacet.sol#L70 Buy emit in auction : https://github.com/kairos-loan/kairos-contracts/blob/ce49230ab5255662d287c4944229cf411725de3f/src/AuctionFacet.sol#L72 Claim emit claimfaucet : https://github.com/kairos-loan/kairos-contracts/blob/ce49230ab5255662d287c4944229cf411725de3f/src/ClaimFacet.sol#L85
Tool used
Manual Review
Recommendation
Use reentrancy guard . Also in RepayFacet.sol
safeTransferFrom()has used but not applied reentrancy guard to this function