veToken Finance contest - GalloDaSballo's results

Lines of code

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

Vulnerability details

Impact

veTokenMinter allows any operator to mint new tokens, that's fine in the context of it being used for:

• having VeAssetDepositor deposit for the user and mint
• Having the Booster mint reward tokens

However because of the open ended system that allows any address to be set as operator, the system allows the admin to set themselves as the operator and to mint an excess amount of tokens, diluting other users.

Because this seems to be used exclusively by the VeAssetDepositor and the Booster hardcoding these two addresses would provide stronger security guarantees

Proof of Concept

addOperator(malicious, {"from": gov})

mint(malicious, AMOUNT, {"from": malicious})

Recommended Mitigation Steps

Set the minters as immutable to provide stronger security guarantees

Make unchanged variable immutable | 2.1k gas

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

rewardFactory is never changed, making it immutable will save 2.1k gas for each time it is read for the first time. This type of gas saving is the biggest you can have.

Avoid setting default value to storage | 100 gas

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

In this constructor call you're setting isShutdown = false;, by default bool values will be false so this assignment will cost extra gas.

This used to save 800 gas but I believe it saves 100 gas now

Needless == true | 3 gas

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

Cache Array length | 3 gas per check

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

Cache var to avoid length check, this will save 3 gas per iteration

Use ++i or ideally unchecked

For loop iterations, you can save 3 gas with pre-increment and almost 20 gas per iteration with unchecked { ++i }

See an example of how to code it here: https://github.com/GalloDaSballo/badger-onchain-rewards/blob/ececa503bcc388786bf68d4e12601c834917ed67/contracts/RewardsManager.sol#L391