Golom contest - CryptoMartian's results

1. Contract rewards/RewardDistributor.sol

[LOW] ignores return value in rewardToken.transfer(...)

File(s): rewards/RewardDistributor.sol

Description: The public functions traderClaim(...), exchangeClaim(..), multiStakerClaim(...) making a transfer call and does not check for return value if the transfer is a success or fails.

function traderClaim(address addr, uint256[] memory epochs) public {
        uint256 reward = 0;
        for (uint256 index = 0; index < epochs.length; index++) {
            require(epochs[index] < epoch);
            reward =
                reward +
                (rewardTrader[epochs[index]] * feesTrader[addr][epochs[index]]) /
                epochTotalFee[epochs[index]];
            feesTrader[addr][epochs[index]] = 0;
        }
        rewardToken.transfer(addr, reward);
    }

function exchangeClaim(address addr, uint256[] memory epochs) public {
        uint256 reward = 0;
        for (uint256 index = 0; index < epochs.length; index++) {
            require(epochs[index] < epoch);
            reward =
                reward +
                (rewardExchange[epochs[index]] * feesExchange[addr][epochs[index]]) /
                epochTotalFee[epochs[index]];
            feesExchange[addr][epochs[index]] = 0;
        }
        rewardToken.transfer(addr, reward);
    }

function multiStakerClaim(uint256[] memory tokenids, uint256[] memory epochs) public {
        require(address(ve) != address(0), ' VE not added yet');

        uint256 reward = 0;
        uint256 rewardEth = 0;
        address tokenowner = ve.ownerOf(tokenids[0]);

        // for each tokenid
        for (uint256 tindex = 0; tindex < tokenids.length; tindex++) {
            require(tokenowner == ve.ownerOf(tokenids[tindex]), 'Can only claim for a single Address together');
            // for each epoch
            for (uint256 index = 0; index < epochs.length; index++) {
                require(epochs[index] < epoch, 'cant claim for future epochs');
                require(claimed[tokenids[tindex]][epochs[index]] == 0, 'cant claim if already claimed');
                claimed[tokenids[tindex]][epochs[index]] = 1;
                if (epochs[index] == 0){
                    rewardEth =
                        rewardEth +
                        (epochTotalFee[0] *
                            ve.balanceOfAtNFT(tokenids[tindex], epochBeginTime[1])) /
                        ve.totalSupplyAt(epochBeginTime[1]);

                }else{
                    reward =
                        reward +
                        (rewardStaker[epochs[index]] * ve.balanceOfAtNFT(tokenids[tindex], epochBeginTime[epochs[index]])) /
                        ve.totalSupplyAt(epochBeginTime[epochs[index]]);
                    rewardEth =
                        rewardEth +
                        (epochTotalFee[epochs[index]] *
                            ve.balanceOfAtNFT(tokenids[tindex], epochBeginTime[epochs[index]])) /
                        ve.totalSupplyAt(epochBeginTime[epochs[index]]);
                }

            }
        }
        rewardToken.transfer(tokenowner, reward);
        weth.transfer(tokenowner, rewardEth);
    }

Recommendation: always check for return value, as in some cases call may return false instead of reverting.

---

2. Contract core/GolomTrader.sol

[Informational] performing multiplication on result of division in function fill_ask(...)

File(s): 'core/GolomTrader.sol`

Description: The function fill_ask(...) is performing multiplication on result of division and it may sometimes lead to loss of precision.

 function fillAsk(
        Order calldata o,
        uint256 amount,
        address referrer,
        Payment calldata p,
        address receiver
    ) public payable nonReentrant {
        // check if the signed total amount has all the amounts as well as 50 basis points fee
        require(
            o.totalAmt >= o.exchange.paymentAmt + o.prePayment.paymentAmt + o.refererrAmt + (o.totalAmt * 50) / 10000,
            'amt not matching'
        );

        // attached ETH value should be greater than total value of one NFT * total number of NFTs + any extra payment to be given
        require(msg.value >= o.totalAmt * amount + p.paymentAmt, 'mgmtm');

        if (o.reservedAddress != address(0)) {
            require(msg.sender == o.reservedAddress);
        }
        require(o.orderType == 0, 'invalid orderType');

        (uint256 status, bytes32 hashStruct, uint256 amountRemaining) = validateOrder(o);

        require(status == 3, 'order not valid');
        require(amountRemaining >= amount, 'order already filled');

        filled[hashStruct] = filled[hashStruct] + amount;

        if (receiver == address(0)) {
            receiver = msg.sender;
        }
        if (o.isERC721) {
            require(amount == 1, 'only 1 erc721 at 1 time');
            ERC721(o.collection).transferFrom(o.signer, receiver, o.tokenId);
        } else {
            ERC1155(o.collection).safeTransferFrom(o.signer, receiver, o.tokenId, amount, '');
        }

        // pay fees of 50 basis points to the distributor
        payEther(((o.totalAmt * 50) / 10000) * amount, address(distributor));

        // pay the exchange share
        payEther(o.exchange.paymentAmt * amount, o.exchange.paymentAddress);

        // pay the pre payment
        payEther(o.prePayment.paymentAmt * amount, o.prePayment.paymentAddress);

        if (o.refererrAmt > 0 && referrer != address(0)) {
            payEther(o.refererrAmt * amount, referrer);
            payEther(
                (o.totalAmt -
                    (o.totalAmt * 50) /
                    10000 -
                    o.exchange.paymentAmt -
                    o.prePayment.paymentAmt -
                    o.refererrAmt) * amount,
                o.signer
            );
        } else {
            payEther(
                (o.totalAmt - (o.totalAmt * 50) / 10000 - o.exchange.paymentAmt - o.prePayment.paymentAmt) * amount,
                o.signer
            );
        }
        payEther(p.paymentAmt, p.paymentAddress);

        distributor.addFee([o.signer, o.exchange.paymentAddress], ((o.totalAmt * 50) / 10000) * amount);
        emit OrderFilled(o.signer, msg.sender, 0, hashStruct, o.totalAmt * amount);
    }

Recommendation: refactor the statement.

[Informational] Missing address(0) check in function setMinter(...)

File(s): https://github.com/code-423n4/2022-07-golom/blob/e5efa8f9d6dda92a90b8b2c4902320acf0c26816/contracts/governance/GolomToken.sol#L58

Description: In function setMinter(...) there is missing input address(0) validation.

function setMinter(address _minter) external onlyOwner {
        pendingMinter = _minter;
        minterEnableDate = block.timestamp + 1 days;
    }

Recommendation: Consider performing zero-address validation before setting the pendingMinter.

---

3. Contract vote-escrow/VoteEscrowDelegation.sol

[Informational] wrong datatype of argument in function signature definition inInterface IVoteEscrow

File(s): vote-escrow/VoteEscrowDelegation.sol

Description: The function definition of balanceOf(...) inside an Interface IVoteEscrow is passing 'uint256' datatype of argument in place of address. The code is shown below.

interface IVoteEscrow {
    function balanceOf(uint256) external view returns (uint256);

    function balanceOfAtNFT(uint256, uint256) external view returns (uint256);

    function ownerOf(uint256) external view returns (address);
}

Recommendation: consider passing the correct datatype of arguments during the function definition.