Velodrome Finance contest - p_crypt0's results

Lines of code

https://github.com/code-423n4/2022-05-velodrome/blob/7fda97c570b758bbfa7dd6724a336c43d4041740/contracts/contracts/Gauge.sol#L173-L186 https://github.com/code-423n4/2022-05-velodrome/blob/7fda97c570b758bbfa7dd6724a336c43d4041740/contracts/contracts/Bribe.sol#L83-L90

Vulnerability details

Impact

Funds drain from Bribe prematurely with repeated calls to deliverBribes()

Proof of Concept

Calling deliverBribes() calls deliverRewards() which transfers the amount specified as the rewards due and sends to gauge.

https://github.com/code-423n4/2022-05-velodrome/blob/7fda97c570b758bbfa7dd6724a336c43d4041740/contracts/contracts/Gauge.sol#L173-L186

https://github.com/code-423n4/2022-05-velodrome/blob/7fda97c570b758bbfa7dd6724a336c43d4041740/contracts/contracts/Bribe.sol#L83-L90

Repeated calls to deliverBribes() makes repeated calls to deliverRewards() with the same amount, since the rewards do not decrease to 0 within the epoch (they reset to 0 when epoch changes).

Recommended Mitigation Steps

Add check to make sure repeated calls can't take place within a set time-zone.

Code Styling

Inconsistent declaration of parameter names in IVELO.sol:

pragma solidity 0.8.13;


interface IVelo {
    function approve(address spender, uint value) external returns (bool);
    function mint(address, uint) external;
    function mintToRedemptionReceiver(uint) external returns (bool);
    function totalSupply() external view returns (uint);
    function balanceOf(address) external view returns (uint);
    function transfer(address, uint) external returns (bool);
    function transferFrom(address,address,uint) external returns (bool);
}

https://github.com/code-423n4/2022-05-velodrome/blob/7fda97c570b758bbfa7dd6724a336c43d4041740/contracts/contracts/interfaces/IVelo.sol#L1-L11

Should read:

pragma solidity 0.8.13;
interface IVelo {
    function approve(address spender, uint value) external returns (bool);
    function mint(address account, uint amount) external;
    function mintToRedemptionReceiver(uint amount) external returns (bool);
    function totalSupply() external view returns (uint);
    function balanceOf(address wallet) external view returns (uint);
    function transfer(address _to, uint _value) external returns (bool);
    function transferFrom(address _from, address _to, uint _value) external returns (bool);
}

[non-critical] Bribe.sol add brackets for readability.

Add an extra bracket for clarity:

uint bribeStart = timestamp - (timestamp % (7 days)) + BRIBE_LAG;

Would read better as:

uint bribeStart = (timestamp - (timestamp % (7 days))) + BRIBE_LAG;

https://github.com/code-423n4/2022-05-velodrome/blob/7fda97c570b758bbfa7dd6724a336c43d4041740/contracts/contracts/Bribe.sol#L36

Add an extra bracket for clarity:

uint bribeEnd = bribeStart + DURATION - COOLDOWN;

Would read better as:

uint bribeEnd = (bribeStart + DURATION) - COOLDOWN;

https://github.com/code-423n4/2022-05-velodrome/blob/7fda97c570b758bbfa7dd6724a336c43d4041740/contracts/contracts/Bribe.sol#L37

[non-critical] Velo.sol does not implement IVELO or IERC20

Velo.sol should implement IVELO interface https://github.com/code-423n4/2022-05-velodrome/blob/7fda97c570b758bbfa7dd6724a336c43d4041740/contracts/contracts/Velo.sol#L4

Contract velo is IVELO {
…
}

Or perhaps implement IVELO and IERC20 for compliance.

Ibribe.sol does not register the event NotifyReward()

(Bribe.sol: https://github.com/code-423n4/2022-05-velodrome/blob/7fda97c570b758bbfa7dd6724a336c43d4041740/contracts/contracts/Bribe.sol#L19

Ibribe.sol: https://github.com/code-423n4/2022-05-velodrome/blob/7fda97c570b758bbfa7dd6724a336c43d4041740/contracts/contracts/interfaces/IBribe.sol#L1-L12)

It does implement the notifyRewardAmount() function.

missing comments

getEpochStart() has no comment: https://github.com/code-423n4/2022-05-velodrome/blob/7fda97c570b758bbfa7dd6724a336c43d4041740/contracts/contracts/Bribe.sol#L34-L39

Add comment:

/**

• getEpochStart
• @notice Gets the start timestamp of the current epoch.
• @param timestamp the current timestamp
• @return uint the start of current epoch of bribes as calculated from timestamp.

*/

notifyRewardAmount() has no comment: https://github.com/code-423n4/2022-05-velodrome/blob/7fda97c570b758bbfa7dd6724a336c43d4041740/contracts/contracts/Bribe.sol#L40-L60

Add comment:

/**

• notifyRewardAmount
• @notice Adds tokens to be rewarded and used for bribes.
• @param token address of token to be added as reward and bribe
• @param amount amount of token to be added for rewards (to be distributed over time). */

rewardsListLength() has no comment: https://github.com/code-423n4/2022-05-velodrome/blob/7fda97c570b758bbfa7dd6724a336c43d4041740/contracts/contracts/Bribe.sol#L61-L64

Add comment:

/**

• rewardsListLength
• @notice returns the length of the current stored rewards list.
• @return rewards_length the length of the current rewards list. */

addRewardToken() has no comment: https://github.com/code-423n4/2022-05-velodrome/blob/7fda97c570b758bbfa7dd6724a336c43d4041740/contracts/contracts/Bribe.sol#L65-L73

Add comment:

/**

• addRewardToken
• @notice Adds a token to the rewards list - must be gauge to add token.
• @param token address of token to be added to rewards */

swapOutRewardToken() has no comment: https://github.com/code-423n4/2022-05-velodrome/blob/7fda97c570b758bbfa7dd6724a336c43d4041740/contracts/contracts/Bribe.sol#L75-L81

Add comment:

/**

• swapOutRewardToken
• @notice Swaps tokens reward status in isReward - must be gauge to swap.
• @param i the index of the token in the rewards list to be swapped.
• @param oldToken the address of the old token.
• @param newToken the address of the new token. */

deliverReward() has no comment: https://github.com/code-423n4/2022-05-velodrome/blob/7fda97c570b758bbfa7dd6724a336c43d4041740/contracts/contracts/Bribe.sol#L82-L90

Add comment:

/**

• deliverReward
• @notice Delivers the accrued rewards of a token to the gauge.
• @param token the address of the token whose rewards will be delivered.
• @param epochStart the start of the epoch */

_safeTransfer() has no comment: https://github.com/code-423n4/2022-05-velodrome/blob/7fda97c570b758bbfa7dd6724a336c43d4041740/contracts/contracts/Bribe.sol#L91-L97

Add comment:

/**

• _safeTransfer
• @notice safely transfers the token to an address from this.
• @param token the address of the token to be transferred.
• @param to the address to send tokens to.
• @param value the amount of the tokens to send to to. */

_safeTransferFrom() has no comment: https://github.com/code-423n4/2022-05-velodrome/blob/7fda97c570b758bbfa7dd6724a336c43d4041740/contracts/contracts/Bribe.sol#L98-L104

Add Comment: /**

• _safeTransferFrom
• @notice safely transfers the token from a specific address, to another address.
• @param token the address of the token to be transferred.
• @param from the address to send tokens from.
• @param to the address to send tokens to.
• @param value the amount of the tokens to send to to. */