Excess ETH Sent By Caller(Buyer) While Buying Players Are Stolen By Owner Of Contract

Summary

Excess Eth sent by caller will lost

Vulnerability Detail

In FootiumAcademy Contract there is function mintPlayers() which used to call by ClubOwner to Buy players from academy. Which calls a private function _validateMintingParams() where it checks ETH send by caller is not less than total players cost

 function _validateMintingParams(
        SeasonID seasonId,
        uint256 clubId,
        uint256 divisionTier,
        uint256[] calldata generationIds,
        bytes32[] calldata divisionProof
    ) private returns (uint256) {
        ....................
        ......................
        uint256 playerCount = generationIds.length;
        uint256 totalFee = playerCount * divisionToFee[divisionTier];
        if (msg.value < totalFee) { // @audit-issue excess eth of user will lost
            revert IncorrectETHAmount(msg.value);
        }

      ..........................................

So if user sent some extra ETH than players net cost then these extra ETH will remain in that Academy contract, and it will withdrawn by Owner via withdraw() function

function withdraw() external onlyOwner {
        uint256 balance = address(this).balance;
        if (balance > 0) {
            (bool sent, ) = payable(owner()).call{value: balance}("");
            if (!sent) {
                revert FailedToSendETH(balance);
            }
        }
    }

Impact

If more Eth sent by Caller(i.e more than players fee), then those extraa eth will be withdrawn by Owner of contract.

Code Snippet

https://github.com/sherlock-audit/2023-04-footium/blob/main/footium-eth-shareable/contracts/FootiumAcademy.sol#L259 https://github.com/sherlock-audit/2023-04-footium/blob/main/footium-eth-shareable/contracts/FootiumAcademy.sol#L207-L212 https://github.com/sherlock-audit/2023-04-footium/blob/main/footium-eth-shareable/contracts/FootiumAcademy.sol#L218-L226

Tool used

Manual Review

Recommendation

Technically if any caller send more eth than sum of all players fee those he intended to Buy, Contract should return extra ETH to that caller

 function mintPlayers(
        SeasonID seasonId,
        uint256 clubId,
        uint256 divisionTier,
        uint256[] calldata generationIds,
        bytes32[] calldata divisionProof
    ) external payable whenNotPaused nonReentrant {
        uint256 totalFee = _validateMintingParams(
            seasonId,
            clubId,
            divisionTier,
            generationIds,
            divisionProof
        );

        uint256 generationId;

        for (uint256 i = 0; i < generationIds.length; ) {
            generationId = generationIds[i];

            if (generationId > _maxGenerationId) {
                revert GenerationIDTooHigh(generationId, _maxGenerationId);
            }

            if (redeemed[seasonId][clubId][generationId]) {
                revert PlayerAlreadyRedeemed(generationId);
            }

            redeemed[seasonId][clubId][generationId] = true;

            uint256 playerId = _footiumPlayer.safeMint(
                _footiumClub.clubToEscrow(clubId)
            );

            emit AcademyPlayerMinted(seasonId, clubId, generationId, playerId);

            unchecked {
                i++;
            }
        }

        // forward total fee to the prize distributor contract
+      uint256 remain = msg.value - totalFee;
        (bool sent, ) = _prizeDistributorAddress.call{value: totalFee}(""); // @audit excess ETH sent by user will lost
        if (!sent) {
            revert FailedToSendETH(totalFee);
        }
+     if(remain != 0){
+          (bool succ, ) = msg.sender.call{value: remain}(""); 
+            require(succ, "BIng Bong, Bing Bong");
+   } 
    }

sherlock-audit / 2023-04-footium-judging

0xhacksmithh - Excess ETH Sent By Caller(Buyer) While Buying Players Are Stolen By Owner Of Contract #387