YOLOrekt / community-bug-bounty

V1 contracts public bounty project
3 stars 1 forks source link

Reentracy #2

Closed Jayfromthe13th closed 2 years ago

Jayfromthe13th commented 2 years ago

Impact Severity: Medium to low Detection of the reentrancy bug. Do not report reentrancies that involve Ether (see reentrancy-eth). Vulnerability Untrusted external contract calls could callback leading to unexpected results such as multiple withdrawals or out-of-order events. Example of vulnerability:

function bug(){
        require(not_called);
        if( ! (msg.sender.call() ) ){
            throw;
        }
        not_called = False;
  }  

Lines affected:Reentrancy in GameInstance.processRound(uint112,uint128,uint128) (contracts/game/GameInstance.sol#298-336): External calls:

Reentrancy in LiquidityPool.mintInitialShares(uint256) (contracts/core/LiquidityPool.sol#135-160): External calls:

Reentrancy in LiquidityPool.mintLpShares(uint256) (contracts/core/LiquidityPool.sol#167-187): External calls:

Reentrancy in WhitelistSFTClaims.claimNft() (contracts/accessory/WhitelistSFTClaims.sol#94-116): External calls:

---Reentrancy in ERC1155SemiFungible._mintNFT(address,uint256,string) (contracts/tokens/extensions/ERC1155SemiFungible.sol#323-360): External calls:

Recommendation Apply the check-effects-interactions pattern and/or reentrancy guard.

CryptoKiddies commented 2 years ago

Thanks for keeping us on the ball! The reentrancy vulnerabilities exist only when a smart contract wallet (or any sc) receives a direct call from another contract caller. None of the call stacks generated in these examples send data directly to a user/smart wallet and therefore are not subject to reentrancy callbacks. Rather, these contracts belong to the YOLO smart contracts system-with the exception of USDC token, and all make state changes within their respective scopes, or at most, calls within YOLOrekt domain. If we were to send data on transfer, then this would become an issue. For example, if we utilized the ERC1155Receiver and sent data to a receiver wallet, then this would become highly relevant.

Regardless, we'll take another look and if changes are needed, there will be a reward coming your way:)