YoloRekt

QuillAudits7 Mar 2022Files named in the report

Audit · firm’s term: audit report

QuillAudits reviewed YoloRekt on 7 Mar 2022: 46 findings, 0 critical; 29 marked fixed, 17 open.

No open critical findings

Open the report Open YoloRekt’s security record

Report hosted on quillaudits.com.

Trustblock does not review code.

Findings46

Statuses are the security firm’s own, from its report.

Severity	Open	Fixed
Critical	0	0
High	3	10
Medium	5	10
Low	9	9

HighAfter expiration time,user does not get any NFT but still has to pay 1 USDCFixed

In the case where a user's expiretime has crossed, this function still takes 1 USDC from the user. Ideally this should not happen as this results in the user paying for nothing as he does not get any NFT and he still has to pay 1 USDC for the same.

HighDecimal ErrorFixed

USDC has 6 decimals and LPool has 18. Means while minting they are not going to get exact amount. For instance, Trying to mint 5 USDC worth of shares will provide only 5e-12 shares, thus creating visualization issues for the users.

HighDividing Rewards and Double HarvestingFixed

As we know releasing funds is a one-time operation that calculates level weightings and intended rewards. However, there exists no check to restrict biddings once the funds are released,As a result, newer bids may extract rewards that were intended for other ids.

HighExploiting the desired 1:1 Yolo:LPShare rate & Possible RugPullsFixed

MINTER_ROLE can mint any number of tokens in order to increase the totalSupply and disrupt the 1:1 share rate.As the mint function doesn’t account for the Yolo Tokens that are supposed to be transferred into users contract and also doesn’t update the userBalances in the users contract. It can lead to multiple exploit scenarios.

HighExploiting usersTokensFixed

The usersTokens holds the token ID of a user. However, it can only hold the most recent token ID, the user has acquired, thus opening doors for overwriting an existing token ID, if the user acquires any new token, which may impact a user in losing a valuable token.

HighIf the funds are already released, any token upgrade thereafter may burn all the rewards.Fixed

While upgrading a token, YoloNFTPack transfers participation units and level weightings from the old token Id to the new token Id. However, if the funds are released or the rewarder contract has been rotated, it means, the rewarder contract in YoloNFTPack has now been replaced with the new rewarder address, as a result, upgrading tokens at this point will call bumpDuringUpgrade of the new rewarder contract, which doesn’t contain any participation units for the old id of this user. If no participation units are being transferred, that means, a user can’t harvest rewards for the new token Id that it has upgraded its old token to. Also, as the user has upgraded its token, the old id has now been burned, i.e the rewards generated from the old id can not be extracted as well. Also, this opportunity could have been utilized to distribute rewards to the remaining participants. However, as the bumpDuringUpgrade was never called for this id on old rewarder contract, the total level weighting will still point to the same weighting, and a portion of the reward will remain unutilized.

HighIncorrect Implementation: The CircusFixed

Function allows anyone to mint initial shares to the system. However, there is no check for the initialAmount, and anyone can call this function with a 0 value for the initialAmount. Minting 0 initial shares will switch the boolean hasLPTokensCirculating to true, even though there is logically no LP Supply in circulation. As the boolean value has now turned to true, it means the contract will now allow to mint more LP shares.But since we minted 0 shares and didn’t transferred any Yolo Tokens to users contract and din’t update the userBalances at users contract. The userBalances still holds 0.Now trying to call mintLpShares, will revert due to divide by 0 panic.

HighResetting usersTokensFixed

Function allows any user to self mint a SemiFungible token of BaseType as BASE_SFT_ID or BaseType 1. However, the function may reset usersTokens,if the user holds a token from another BaseType, and may impact a huge loss to the token holder.

HighResetting values incorrectly while burning a token IDFixed

Function burn allows a user to burn its token ID. However, the function doesn’t account for the scenarios where the operator can call this function on behalf of the token ID holder, as a consequence, the operator may reset its own usersTokens and _hasNFTType, instead of the concerned account.

HighUse of SafeTransfer for Third party Token ContractFixed

Use safeTransferFrom for token transfers as USDC is an upgradeable contract, or check return values of token transfers. Needs to be fixed for every instance in every contract.

Show 10 more

MediumCentralization of processRound:Fixed

processRound() function is used to make the round live so that it cannot accept furrther bids and give winning players their payout. But this can only be called by GAME_ADMIN. This could resulting in the admin denying the players their payouts for a long time and thus resulting in Denial of Service.

MediumIncorrect Implemented Logic to calculate Level RewardsFixed

The intended logic of the function is to allow users to view expected level rewards at any point in time. However, the function logic doesn’t consider multiplying rewardsMultiplier for the concerning level and thus will produce incorrect results.For the same reason, getUserPendingReward will produce incorrect results as it depends upon getLatestLevelReward to calculate the level’s reward.

MediumMissing Address ValidationFixed

Function allows Admins to register a new contract to the Yolo Ecosystem. However, there exists no input validation for the contract address to be registered, as a result, address collisions may happen, or in other words, the same address can be registered for multiple contract identifiers. Example Scenario: Registering, the same address for two different contract identifiers for USERS and YOLO_TOKEN, which may lead to unexpected behavior, as all yolo ecosystem’s contracts rely upon the yoloRegistry for the contract initializations and other references/operations.

MediumMissing checks may lead to incorrect contract initializationFixed

A different pattern has been observed in contrast with other contracts which refer to YoloRegistry for registered contract addresses. However, here, the nftTrackerContract and yoloNFTPackContract are being initialized without doing that, and therefore, are prone to incorrect initializations as they may be set to zero addresses or incorrect addresses.

MediumMissing value checks on startTime parameterFixed

startTime parameter of processRound() function has no upper limit and can be set incorrectly as very long time in future. This could result in a potential Denial of Service.

MediumMutable URIsFixed

Function allows Authorities to modify URI of any SemiFungible or NonFungible id at any point in time. Authorities with malicious intent may use the function to exploit the characteristics/traits of a token.

MediumStableCoinToken is supposed to be an interfaceFixed

ERC20 public immutable stablecoinTokenContract;

MediumTricking the system to hold more than 1 token from same basetypeFixed

The logical requirement of the team/yolo ecosystem to not allow users to have more than 1 token from same basetype. But that can be tricked with the help of safeTransferFrom and safeBatchTransferFrom function.

MediumUntitled findingFixed

The logical requirement is that liquidity providers should maintain a minimum amount of 400 USDC. However, they can bypass it by burning down the shares, which they don't require, thus tricking the system and avoding the minimum requirement.

MediumUpdating Liquidity Pool Balance incorrectlyFixed

allows Authorized roles to update userBalances for Liquidity Pool. However, the function updates the balances for msg.sender and not LP address, which means it won’t be possible to manually intervene and update the userBalances for Liquidity Pool in order to maintain the required 1:1 share rate for Yolo and LP shares if needed. Because, if called by any authorized role, the function will be updating userBalances for the authorized roles itself and not for the Liquidity Pool.

Show 9 more

LowLogical Code Optimization#1Fixed

While setting Level Requirement, there is no need for the checks mentioned above, as for the first level, there will be no previous level(can be considered level 0), meaning prevLevel.bidCountThreshold and prevLevel.cumulativeAmountThreshold will always be 0. Will be taking care of the values to be greater than 0. So, the first level can have a minimum value 1 for the bidCountThreshold and cumulativeAmountThreshold. Also, as the check makes sure that the new level’s bidCountThreshold and cumulativeAmountThreshold stay greater than the previous level, the following levels will always have non-zero value for bidCountThreshold and cumulativeAmountThreshold.

LowLogical Code Optimization#2Fixed

Function can be optimized to add a non-zero value check for parameter length, as it doesn’t make any sense to return 0 elements from any starting index.

LowNon-Executable CodeFixed

Multiple instances of non-executable code have been reported, which may be removed to increase the code readability and decrease code size.

LowPossibly incorrect Error StatementFixed

Harvesting of rewards are supposed to be done once the funds are released. Hence, the statement may be changed to state that the “funds must be released” in order to avoid any confusions.

LowRedundant CheckFixed

constructor adds a Zero Address check for registryContractAddress_ initialization. However, RegistrySatellite already considers the same check,thus making the check, in Users contract redundant, and may be removed.

LowSpam MintingFixed

Function allows any user to self mint a SemiFungible token of BaseType as BASE_SFT_ID or BaseType 1. However, no access control may lead to spam minting.

LowUnsafe DownCasting and “uint” Space MismatchFixed

Function updateTracking allows the registered game contracts to update NFT/LEVEL tracking details. However, at #L161, the function performs an unsafe downcasting from uint192 to 160, which may lead to incorrect details being updated. Also, theNftData holds a type uint192 for cumulativeBidAmount, whereas LevelTracking holds a type uint176 for totalCumulativeBidAmount,Which means, cumulativeBidAmount of an NFT can even exceed the totalCumulativeBidAmount of the level, which seems logically incorrect.

LowUnused CodeFixed

Contract defines a mapping as registeredAssets, which has not been used throughout the audit scope.

LowUnused MARKET_MAKER_ROLEFixed

The contract initialization grants MARKET_MAKER_ROLE to the deployer. However, it is not being used for any Access Control, for the scope of the contract.