veToken Finance contest - Hawkeye's results

When a user stakes an amount, this value gets added to the totalSupply and on withdrawal, the opposite is done.The updateReward modifier does a series of calculation to ascertain the rewards due to a specific account. Notable values updated are rewardPerTokenStored and rewards[account].These are derived as such :

https://github.com/code-423n4/2022-05-vetoken/blob/2d7cd1f6780a9bcc8387dea8fecfbd758462c152/contracts/BaseRewardPool.sol#L157-L159

https://github.com/code-423n4/2022-05-vetoken/blob/2d7cd1f6780a9bcc8387dea8fecfbd758462c152/contracts/BaseRewardPool.sol#L166-L169

earned() utilises the returned value of RewardPerToken () to obtain the amount earned but this will be flawed. This is due in part to the fact that in stakeFor (), a malicious user can set address(_for) to the BaseRewardPool which will therefore inflate the supply and result in lowered rewards for the user. One might think that attackers’ main impetus is economical but in some instances, it can be due to malicious intent, ie, just causing inconveniences to a group. Now, it is understandable that users’ reward is reduced on the occasion that the totalSupply increases, but this is only acceptable when it is done in a manner that is consistent with the logic written. If a malicious individual forcibly stakes for the contract in question, the supply never gets reduced as the code dictates(through withdraw() which will never occur since the msg.sender ==address(this)).

To illustrate an example, if at a given moment, users had staked and withdrawn and totalSupply should be 10100 but is 11000, that's a 8.2% reduction in rewards.

Use a require statement to ensure that _for != address(this).

It is understood that the protocol wants to allow for one user to stake on behalf of another. But an event like this (not impossible ) could occur.

#0 - GalloDaSballo
2022-07-07T22:59:09Z

Check that the lengths of the arrays match

Valid Refactor

## Variable types are already their default NC

## Unnecessary use of SafeMath at various parts of the codebase since solidity 0.8.0 checks for overflow and underflow et al Valid R

Maintain uniformity in how constant variables are written

Valid R, nice catch

## Utilise addition /subtraction assignment for better readability I don't understand this finding, cannot give it points, please always add a description to your findings as the context is as important as the link to the code

Users’ rewards can be lowered if totalSupply is inflated unnaturally

Nice finding, valid Low, may bump up if it was found by others

Neat report

1L 2R 1NC

Findings Information

🌟 Selected for report: IllIllI

Also found by: 0x1f8b, 0x29A, 0xKitsune, 0xNazgul, 0xf15ers, 0xkatana, Cityscape, Dravee, ElKu, FSchmoede, Funen, GalloDaSballo, Hawkeye, Kaiziron, MiloTruck, Randyyy, RoiEvenHaim, Ruhum, SecureZeroX, SmartSek, TerrierLover, TomJ, Tomio, WatchPug, Waze, _Adam, asutorufos, c3phas, catchup, cogitoergosumsw, delfin454000, ellahi, fatherOfBlocks, gzeon, hansfriese, horsefacts, jonatascm, minhquanym, oyc_109, pauliax, reassor, robee, sach1r0, saian, sashik_eth, simon135, z3s

Awards

52.0224 USDT - $52.02

Labels

bug

G (Gas Optimization)

External Links

Link to GitHub issue

Gas

For small gas savings (little goes a long way)

Within the “if” and “require” statement (which necessitates a condition), for readability and to save gas, remove the explicit use of the Boolean terms.

VoterProxy.sol

https://github.com/code-423n4/2022-05-vetoken/blob/main/contracts/VoterProxy.sol#L93-L97

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

Booster.sol

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

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

For eg, Use if(!x) or(x) {

…..

}

and require(x) or (! x)

Use unchecked block

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

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

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

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

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

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

Check whether a non-zero value exists before accessing the state variable

Below, totalWeight is being decremented before anything is added. This would be waste gas when an address is being added for the first time without any weight :

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

Use an if statement prior to reducing the totalWeight which checks whether the specific asset has any weight.

For eg, if (veAssetWeight[_asset] >0){

……

}

#0 - GalloDaSballo
2022-07-14T02:10:53Z

Effectively just a recommendation to use the unchecked block, without the math nor further details.

Would save less than 500 gas overall

Considering marking invalid against 50+ submissions

#1 - GalloDaSballo
2022-07-26T23:57:29Z

Downgrading to 100 gas saved, but maintaing as valid

veToken Finance contest - Hawkeye's results

Lock more veAsset permanently.

General Information

Findings Distribution

Researcher Performance

External Links

[Q-28] QA Report - $103.42

Findings Information

Awards

Labels

External Links

QA

Check that the lengths of the arrays match

Variable types are already their default

Unnecessary use of SafeMath at various parts of the codebase since solidity 0.8.0 checks for overflow and underflow et al

Maintain uniformity in how constant variables are written

Utilise addition /subtraction assignment for better readability

Users’ rewards can be lowered if totalSupply is inflated unnaturally

#0 - GalloDaSballo2022-07-07T22:59:09Z

Check that the lengths of the arrays match

Maintain uniformity in how constant variables are written

Users’ rewards can be lowered if totalSupply is inflated unnaturally

[G-11] Gas Report - $52.02

Findings Information

Awards

Labels

External Links

Gas

Use unchecked block

Check whether a non-zero value exists before accessing the state variable

#0 - GalloDaSballo2022-07-14T02:10:53Z

#1 - GalloDaSballo2022-07-26T23:57:29Z

#0 - GalloDaSballo
2022-07-07T22:59:09Z

#0 - GalloDaSballo
2022-07-14T02:10:53Z

#1 - GalloDaSballo
2022-07-26T23:57:29Z