code-423n4 / 2022-04-jpegd-findings

1 stars 1 forks source link

QA Report #217

Open code423n4 opened 2 years ago

code423n4 commented 2 years ago

LOW

1 No check that totalStake >= _amount

https://github.com/code-423n4/2022-04-jpegd/blob/main/contracts/farming/yVaultLPFarming.sol#L117-L130 As the function check that balanceOf[msg.sender] >= _amount, the function should check that totalStaked >= _amount. Then use unchecked for calculation of balanceOf and totalStaked for gas optimization:

    function withdraw(uint256 _amount) external noContract(msg.sender) {
        require(_amount > 0, "invalid_amount");
        require(balanceOf[msg.sender] >= _amount, "insufficient_amount");
    require(totalStaked >= _amount, "insufficient_amount");

        _update();
        _withdrawReward(msg.sender);

        Unchecked{
        balanceOf[msg.sender] -= _amount;
            totalStaked -= _amount;
    }
        vault.safeTransfer(msg.sender, _amount);

        emit Withdraw(msg.sender, _amount);
    }

2 Comment line might mismatch with the code

https://github.com/code-423n4/2022-04-jpegd/blob/main/contracts/lock/JPEGLock.sol#L44 In case owner has reset the lockTime through setLockTime function, The comment won't match the code anymore Change it to:

    /// @notice Locks `_lockAmount` tokens for account `_account` and NFT `_nftIndex` for lockTime duration. 

3 Unnecessary _newTime check in setLockTime function

https://github.com/code-423n4/2022-04-jpegd/blob/main/contracts/lock/JPEGLock.sol#L40 In case owner set _newTime = 1, the check won't revert anything and it has almost the same impact as _newTime = 0 (its just 1 sec different).

RECOMMENDED MITIGATION Set the minimum time for lockTime duration or just remove the check for gas opt

4 No check that _lockAmount != 0

https://github.com/code-423n4/2022-04-jpegd/blob/main/contracts/lock/JPEGLock.sol#L54 IMPACT Owner can just push positions mapping with 0 amount nft and nothing to unlock with it. It might just fill positions with nothing