code423n4 commented 2 years ago

Lines of code

https://github.com/code-423n4/2022-05-vetoken/blob/2d7cd1f6780a9bcc8387dea8fecfbd758462c152/contracts/VeTokenMinter.sol#L77

Vulnerability details

Impact

Compromised owner can withdraw() entire balance of VeTokenMinter.sol to any other account.

Proof of Concept

https://github.com/code-423n4/2022-05-vetoken/blob/2d7cd1f6780a9bcc8387dea8fecfbd758462c152/contracts/VeTokenMinter.sol#L77-L81

function withdraw(address _destination, uint256 _amount) external onlyOwner {
    veToken.safeTransfer(_destination, _amount);

    emit Withdraw(_destination, _amount);
}

The owner can choose any _destination and _amount to send funds to with no delay or limit. These funds could be used to call Booster.deposit() and then Booster.withdraw()(withdraw) the equivalent in lptoken.

Tools Used

Manual Review

Recommended Mitigation Steps

Consider implementing a timelock on VeTokenMinter.withdraw() and changing the destination to an address that owner has no control over.

Example of similar issues illustrating the severity of the finding can be found here (H-09).

solvetony commented 2 years ago

Requires compromised owner.

GalloDaSballo commented 2 years ago

Finding is valid, but contingent on a Malicious or Compromised Admin, Medium Severity is more appropriate

code-423n4 / 2022-05-vetoken-findings