XYRO - audits

In the `closeGame()` function, as long as the game state is "Created" or the current time is three days after the game's `endTime`, an attacker can call `closeGame()` to refund the initiator's deposit repeatedly: `OneVsOneExactPrice.sol` ```solidity=405 function closeGame(bytes32 gameId) public { GameInfo memory game = decodeData(gameId); require(game.initiator == msg.sender, "Wrong sender"); require( game.gameStatus == Status.Created || ( block.timestamp > game.endTime ? block.timestamp - game.endTime >= 3 days : false ), "Wrong status!" ); ITreasury(treasury).refund( games[gameId].depositAmount, game.initiator, gameId ); //rewrites status games[gameId].packedData2 = (games[gameId].packedData2 & ~(uint256(0xFF) << 208)) | (uint256(uint8(Status.Cancelled)) << 208); emit ExactPriceCancelled(gameId); } ``` As a result, when the current time is three days after the `endTime`, there is no need to check the game state, and the refund can be made to the initiator. This allows `closeGame()` to be called repeatedly.For games in the "started" state, repeated calls by the initiator will also "refund" the opponent's deposit amount to initiator. In addition, when the game's lock record in treasury is large enough, the lock amount can be continuously withdrawn through this function. By combining this with the vulnerability in `createGameWithPermit`, which allows the repeated creation of a game with the same `gameId`, low-value tokens can be exchanged for high-value tokens.

In the `UpDown` contract, the `finalizeGame` function is responsible for finalizing the game and distributing rewards. However, when the game is canceled due to either `UpPlayers.length == 0` or `DownPlayers.length == 0`, the function resets several global storage variables but fails to reset `totalRakebackUp` and `totalRakebackDown`. This oversight can lead to incorrect calculations and potential asset loss in subsequent game rounds. The critical section of the code is: ```solidity if (UpPlayers.length == 0 || DownPlayers.length == 0) { // ... refund logic ... packedData = 0; totalDepositsUp = 0; totalDepositsDown = 0; currentGameId = bytes32(0); return; } // ... other logic ... delete DownPlayers; emit UpDownCancelled(currentGameId); packedData = 0; currentGameId = bytes32(0); return; // ... other logic ... delete DownPlayers; delete UpPlayers; ITreasury(treasury).setGameFinished(currentGameId); currentGameId = bytes32(0); packedData = 0; totalDepositsUp = 0; totalDepositsDown = 0; ``` The same issue is present at the end of the `finalizeGame` function where `totalRakebackUp` and `totalRakebackDown` are not reset, potentially affecting future games.

In this function, there is no check like the one in `createGame` and `createGameWithDeposit` where `require(games[gameId].packedData == 0, "Game exists")` is used to verify if the current `gameId` already exists. As a result, when two `createGameWithPermit` transactions with the same `endTime` and both being public games (opponent is `address(0)`) are processed in the same block, the first created game will be overwritten by the second. Consequently, the user's bet for the first game will be locked in the contract and cannot be refunded or withdrawn through the game. In addition, since the locked amount for the game corresponding to the `gameId` has increased before the overwrite, this can be exploited in combination with the vulnerability that allows `closeGame()` to be called multiple times. This can lead to the exchange of low-value tokens for high-value tokens.

In the case where `UpPlayers.length == 0 || DownPlayers.length == 0`, the game is invalid, and the game principal should be refunded to the players, followed by a return. However, before the return, `startingPrice` is not set to zero: `UpDown.sol` ```solidity=304 emit UpDownCancelled(currentGameId); packedData = 0; totalDepositsUp = 0; totalDepositsDown = 0; totalRakebackUp = 0; totalRakebackDown = 0; currentGameId = bytes32(0); return; ``` This will result in an existing `startingPrice` before the next game starts.

When `uint192(finalPrice / 1e14) == _game.startingPrice`, after refunding the tokens to the user, the user's `depositAmounts` are not set to 0, and `totalDepositsUp` and `totalDepositsDown` are not reset to 0: `UpDown.sol` ```solidity=348 else if (uint192(finalPrice / 1e14) == _game.startingPrice) { for (uint i; i < UpPlayers.length; i++) { ITreasury(treasury).refund( depositAmounts[UpPlayers[i]], UpPlayers[i], currentGameId ); isParticipating[UpPlayers[i]] = false; } delete UpPlayers; for (uint i; i < DownPlayers.length; i++) { ITreasury(treasury).refund( depositAmounts[DownPlayers[i]], DownPlayers[i], currentGameId ); isParticipating[DownPlayers[i]] = false; } delete DownPlayers; emit UpDownCancelled(currentGameId); packedData = 0; currentGameId = bytes32(0); return; } ``` This will cause the variables from the current round to be included in the next round's game, leading to confusion. It may result in the `finalizeGame()` function for the next game being unable to execute.

In the `Treasury` contract, the `distributeBullseye` function is responsible for deducting the appropriate rakeback for each guess during reward distribution. However, if a player participates multiple times in a single round, the rakeback calculation may be incorrect. For instance, if userA participates twice with different guesses and userB participates once, and userA's second guess wins, both userA and userB are meant to retain one share of rakeback, with userA winning the reward. The issue arises when calculating `winnersRakeback`, as all of userA's rakeback entries are considered, including those from losing guesses. This results in insufficient rakeback for players to withdraw. ```solidity for (uint i = 0; i < 3; i++) { if (currentRates[i] != 0) { winnersRakeback += ITreasury(treasury).lockedRakeback( currentGameId, topPlayers[i] ); } } for (uint256 i = 0; i < 3; i++) { if (topPlayers[i] != address(0)) { if (currentRates[i] != 0) { ITreasury(treasury).distributeBullseye( currentRates[i], totalRakeback - winnersRakeback, topPlayers[i], currentGameId, topRakeback[i] ); } } } ```

The `playWithPermit` function of `Bullseye` contract incorrectly calculates and double-counts the rakeback. The line `uint256 rakeback = totalRakeback += ITreasury(treasury).depositAndLockWithPermit(...)` both assigns and adds the value of rakeback to `totalRakeback`, leading to an incorrect rakeback value being stored in `playerGuessData` and emitted in the `BullseyeNewPlayer` event. ```solidity function playWithPermit( uint256 assetPrice, ITreasury.PermitData calldata permitData ) public { ... @> uint256 rakeback = totalRakeback += ITreasury(treasury) .depositAndLockWithPermit( depositAmount, msg.sender, currentGameId, playerGuessData.length, permitData.deadline, permitData.v, permitData.r, permitData.s ); ... @> totalRakeback += rakeback; emit BullseyeNewPlayer( msg.sender, assetPrice, depositAmount, currentGameId, playerGuessData.length - 1, rakeback ); } ```

The `lock` function is responsible for locking tokens for a game and is invoked by other functions like `playWithDeposit` and `createGameWithDeposit`. However, unlike the deposit functions `depositAndLock`, `depositAndLockWithPermit` etc , `lock` does not enforce the requirement that the amount must be greater than or equal to `minDepositAmount[token]`. This omission allows players to lock fewer tokens than intended, potentially undermining game mechanics. Below is the relevant code snippet: ```solidity function lock( uint256 amount, address from, bytes32 gameId, bool isRakeback ) public onlyRole(DISTRIBUTOR_ROLE) returns (uint256 rakeback) { address token = gameToken[gameId]; require(approvedTokens[token], "Unapproved token"); require(deposits[token][from] >= amount, "Insufficent deposit amount"); // Missing minDepositAmount[token] check if (isRakeback) { rakeback = calculateRakebackAmount(from, amount); lockedRakeback[gameId][from] += rakeback; } deposits[token][from] -= amount; locked[gameId] += amount; } ```

The `fee` variables in the contract have no set limits, allowing fees to potentially reach 100%. If this occurs, users would receive no tokens from the contract, resulting in a complete loss of their investment.

In the `OneVsOneExactPrice` contract, the `closeGame` function allows the game's initiator to close the game and refund tokens when the game is in the `Created` status or when the current time is three days after `game.endTime`. However, if the game is `Started`, the function only refunds the initiator's assets and does not account for the opponent's assets. Furthermore, there are no other functions available to refund the opponent's assets, potentially causing their assets to be locked indefinitely. The relevant code snippet is: ```solidity function closeGame(bytes32 gameId) public { GameInfo memory game = decodeData(gameId); require(game.initiator == msg.sender, "Wrong sender"); require( game.gameStatus == Status.Created || ( block.timestamp > game.endTime ? block.timestamp - game.endTime >= 3 days : false ), "Wrong status!" ); ITreasury(treasury).refund( games[gameId].depositAmount, game.initiator, gameId ); //rewrites status games[gameId].packedData2 = (games[gameId].packedData2 & ~(uint256(0xFF) << 208)) | (uint256(uint8(Status.Cancelled)) << 208); emit ExactPriceCancelled(gameId); } ```