Golom contest - ladboy233's results

Lines of code

https://github.com/code-423n4/2022-07-golom/blob/e5efa8f9d6dda92a90b8b2c4902320acf0c26816/contracts/core/GolomTrader.sol#L154

Vulnerability details

Impact

Detailed description of the impact of this finding.

transfer() and send() should be avoided (because they take a hard dependency on gas costs by forwarding a fixed amount of gas: 2300).

call{value: ...}("") should be used, for example: contractB.call{value: 1000}("")

Proof of Concept

Provide direct links to all referenced code in GitHub. Add screenshots, logs, or any other relevant proof that illustrates the concept.

Any gas specific code should be avoided because gas costs can and will change.

For example, the gas cost for SLOAD was raised from 50 to 200 in 2016, and again in EIP 1884, with some impacts described in: https://chainsecurity.com/istanbul-hardfork-eips-increasing-gas-costs-and-more/

reference

https://ethereum.stackexchange.com/questions/78124/is-transfer-still-safe-after-the-istanbul-update

Tools Used

Recommended Mitigation Steps

use .call(value) instead of transfer.

abi.encode() is less efficient than abi.encodePacked()

can use abi.encodePacked() instead of abi.encode() to save gas.

https://github.com/code-423n4/2022-07-golom/blob/e5efa8f9d6dda92a90b8b2c4902320acf0c26816/contracts/core/GolomTrader.sol#L102

 EIP712_DOMAIN_TYPEHASH = keccak256(
            abi.encode(
                keccak256('EIP712Domain(string name,string version,uint256 chainId,address verifyingContract)'),
                keccak256(bytes('GOLOM.IO')),
                keccak256(bytes('1')),
                chainId,
                address(this)
            )
);

https://github.com/code-423n4/2022-07-golom/blob/e5efa8f9d6dda92a90b8b2c4902320acf0c26816/contracts/core/GolomTrader.sol#L128

        return
            keccak256(
                abi.encode(
                    keccak256(
                        'order(address collection,uint256 tokenId,address signer,uint256 orderType,uint256 totalAmt,payment exchange,payment prePayment,bool isERC721,uint256 tokenAmt,uint256 refererrAmt,bytes32 root,address reservedAddress,uint256 nonce,uint256 deadline)payment(uint256 paymentAmt,address paymentAddress)'
                    ),

https://github.com/code-423n4/2022-07-golom/blob/e5efa8f9d6dda92a90b8b2c4902320acf0c26816/contracts/core/GolomTrader.sol#L414

  bytes32 computedHash = keccak256(abi.encode(leaf));

modifier can be marked as private instead of internal

https://github.com/code-423n4/2022-07-golom/blob/e5efa8f9d6dda92a90b8b2c4902320acf0c26816/contracts/core/GolomTrader.sol#L380

 function _settleBalances(
        Order calldata o,
        uint256 amount,
        address referrer,
        Payment calldata p
    ) internal 

internal can be marked private.

modifiers can be marked as external instead of public

https://github.com/code-423n4/2022-07-golom/blob/e5efa8f9d6dda92a90b8b2c4902320acf0c26816/contracts/core/GolomTrader.sol#L203

 function fillAsk(
        Order calldata o,
        uint256 amount,
        address referrer,
        Payment calldata p,
        address receiver
    ) public payable nonReentrant 

https://github.com/code-423n4/2022-07-golom/blob/e5efa8f9d6dda92a90b8b2c4902320acf0c26816/contracts/core/GolomTrader.sol#L279

public can be marked as external

    function fillBid(
        Order calldata o,
        uint256 amount,
        address referrer,
        Payment calldata p
    ) public nonReentrant {

https://github.com/code-423n4/2022-07-golom/blob/e5efa8f9d6dda92a90b8b2c4902320acf0c26816/contracts/core/GolomTrader.sol#L334

 function fillCriteriaBid(
        Order calldata o,
        uint256 amount,
        uint256 tokenId,
        bytes32[] calldata proof,
        address referrer,
        Payment calldata p
    ) public nonReentrant {

https://github.com/code-423n4/2022-07-golom/blob/e5efa8f9d6dda92a90b8b2c4902320acf0c26816/contracts/rewards/RewardDistributor.sol#L98

function addFee(address[2] memory addr, uint256 fee) public onlyTrader

https://github.com/code-423n4/2022-07-golom/blob/e5efa8f9d6dda92a90b8b2c4902320acf0c26816/contracts/rewards/RewardDistributor.sol#L141

 function traderClaim(address addr, uint256[] memory epochs) public {

https://github.com/code-423n4/2022-07-golom/blob/e5efa8f9d6dda92a90b8b2c4902320acf0c26816/contracts/rewards/RewardDistributor.sol#L155

function exchangeClaim(address addr, uint256[] memory epochs) public {

https://github.com/code-423n4/2022-07-golom/blob/e5efa8f9d6dda92a90b8b2c4902320acf0c26816/contracts/rewards/RewardDistributor.sol#L155

function multiStakerClaim(uint256[] memory tokenids, uint256[] memory epochs) public {

https://github.com/code-423n4/2022-07-golom/blob/e5efa8f9d6dda92a90b8b2c4902320acf0c26816/contracts/rewards/RewardDistributor.sol#L215

 function stakerRewards(uint256 tokenid) public view returns (

https://github.com/code-423n4/2022-07-golom/blob/e5efa8f9d6dda92a90b8b2c4902320acf0c26816/contracts/rewards/RewardDistributor.sol#L254

function traderRewards(address addr) public view returns

https://github.com/code-423n4/2022-07-golom/blob/e5efa8f9d6dda92a90b8b2c4902320acf0c26816/contracts/rewards/RewardDistributor.sol#L269

function exchangeRewards(address addr) public view returns

USE CUSTOM ERRORS REVERT()/REQUIRE() to SAVE GAS

Custom errors are available from solidity version 0.8.4. Custom errors save ~50 gas each time they’re hitby avoiding having to allocate and store the revert string. Not defining the strings also save deployment gas

https://github.com/code-423n4/2022-07-golom/blob/e5efa8f9d6dda92a90b8b2c4902320acf0c26816/contracts/core/GolomTrader.sol#L177

require(signaturesigner == o.signer, 'invalid signature');

https://github.com/code-423n4/2022-07-golom/blob/e5efa8f9d6dda92a90b8b2c4902320acf0c26816/contracts/core/GolomTrader.sol#L211

require(
            o.totalAmt >= o.exchange.paymentAmt + o.prePayment.paymentAmt + o.refererrAmt + (o.totalAmt * 50) / 10000,
            'amt not matching'
        );

https://github.com/code-423n4/2022-07-golom/blob/e5efa8f9d6dda92a90b8b2c4902320acf0c26816/contracts/core/GolomTrader.sol#L217

require(msg.value >= o.totalAmt * amount + p.paymentAmt, 'mgmtm');

https://github.com/code-423n4/2022-07-golom/blob/e5efa8f9d6dda92a90b8b2c4902320acf0c26816/contracts/core/GolomTrader.sol#L222

require(o.orderType == 0, 'invalid orderType');

https://github.com/code-423n4/2022-07-golom/blob/e5efa8f9d6dda92a90b8b2c4902320acf0c26816/contracts/core/GolomTrader.sol#L226

 require(status == 3, 'order not valid');

https://github.com/code-423n4/2022-07-golom/blob/e5efa8f9d6dda92a90b8b2c4902320acf0c26816/contracts/core/GolomTrader.sol#L226

require(amountRemaining >= amount, 'order already filled');

https://github.com/code-423n4/2022-07-golom/blob/e5efa8f9d6dda92a90b8b2c4902320acf0c26816/contracts/core/GolomTrader.sol#L235

https://github.com/code-423n4/2022-07-golom/blob/e5efa8f9d6dda92a90b8b2c4902320acf0c26816/contracts/core/GolomTrader.sol#L299

https://github.com/code-423n4/2022-07-golom/blob/e5efa8f9d6dda92a90b8b2c4902320acf0c26816/contracts/core/GolomTrader.sol#L359

https://github.com/code-423n4/2022-07-golom/blob/e5efa8f9d6dda92a90b8b2c4902320acf0c26816/contracts/core/GolomTrader.sol#L455

 require(amount == 1, 'only 1 erc721 at 1 time');

https://github.com/code-423n4/2022-07-golom/blob/e5efa8f9d6dda92a90b8b2c4902320acf0c26816/contracts/rewards/RewardDistributor.sol#L173

require(address(ve) != address(0), ' VE not added yet');

https://github.com/code-423n4/2022-07-golom/blob/e5efa8f9d6dda92a90b8b2c4902320acf0c26816/contracts/rewards/RewardDistributor.sol#L181

··· require(tokenowner == ve.ownerOf(tokenids[tindex]), 'Can only claim for a single Address together'); ···

https://github.com/code-423n4/2022-07-golom/blob/e5efa8f9d6dda92a90b8b2c4902320acf0c26816/contracts/rewards/RewardDistributor.sol#L184

··· require(epochs[index] < epoch, 'cant claim for future epochs'); require(claimed[tokenids[tindex]][epochs[index]] == 0, 'cant claim if already claimed'); ···

https://github.com/code-423n4/2022-07-golom/blob/e5efa8f9d6dda92a90b8b2c4902320acf0c26816/contracts/rewards/RewardDistributor.sol#L220

··· require(address(ve) != address(0), ' VE not added yet'); ···

https://github.com/code-423n4/2022-07-golom/blob/e5efa8f9d6dda92a90b8b2c4902320acf0c26816/contracts/rewards/RewardDistributor.sol#L292

··· require(traderEnableDate <= block.timestamp, 'RewardDistributor: time not over yet'); ···

https://github.com/code-423n4/2022-07-golom/blob/e5efa8f9d6dda92a90b8b2c4902320acf0c26816/contracts/rewards/RewardDistributor.sol#L309

··· require(voteEscrowEnableDate <= block.timestamp, 'RewardDistributor: time not over yet'); ···

Using bools for storage incurs overhead

https://github.com/code-423n4/2022-07-golom/blob/e5efa8f9d6dda92a90b8b2c4902320acf0c26816/contracts/governance/GolomToken.sol#L20

https://github.com/code-423n4/2022-07-golom/blob/e5efa8f9d6dda92a90b8b2c4902320acf0c26816/contracts/governance/GolomToken.sol#L21

bool public isAirdropMinted;

 bool public isGenesisRewardMinted;

we can use uint8, 0 or 1 to indicate the state change.

can cache rewardTokn.totalSupply() and rewardToken.balanceOf(address(ve))

https://github.com/code-423n4/2022-07-golom/blob/e5efa8f9d6dda92a90b8b2c4902320acf0c26816/contracts/rewards/RewardDistributor.sol#L112

uint256 tokenToEmit = (dailyEmission * (rewardToken.totalSupply() - rewardToken.balanceOf(address(ve)))) /
                rewardToken.totalSupply();
uint256 stakerReward = (tokenToEmit * rewardToken.balanceOf(address(ve))) / rewardToken.totalSupply();

rewardTokn.totalSupply() and rewardToken.balanceOf(address(ve)) are called muptiple times. both can be cached in uint256 variable to save gas. such as

uint256 totalSupply = rewardTokn.totalSupply()
uint256 veBalance = rewardToken.balanceOf(address(ve))

<array>.length should not be looked up in every loop of a for-loop

https://github.com/code-423n4/2022-07-golom/blob/e5efa8f9d6dda92a90b8b2c4902320acf0c26816/contracts/rewards/RewardDistributor.sol#L141

https://github.com/code-423n4/2022-07-golom/blob/e5efa8f9d6dda92a90b8b2c4902320acf0c26816/contracts/rewards/RewardDistributor.sol#L155

<array>.length can be cached.

https://github.com/code-423n4/2022-07-golom/blob/e5efa8f9d6dda92a90b8b2c4902320acf0c26816/contracts/rewards/RewardDistributor.sol#L143

https://github.com/code-423n4/2022-07-golom/blob/e5efa8f9d6dda92a90b8b2c4902320acf0c26816/contracts/rewards/RewardDistributor.sol#L157

for (uint256 index = 0; index < epochs.length; index++) {

epochs.length can be cached.

https://github.com/code-423n4/2022-07-golom/blob/e5efa8f9d6dda92a90b8b2c4902320acf0c26816/contracts/rewards/RewardDistributor.sol#L180

for (uint256 tindex = 0; tindex < tokenids.length; tindex++) {