Platform: Code4rena
Start Date: 23/05/2022
Pot Size: $75,000 USDC
Total HM: 23
Participants: 75
Period: 7 days
Judge: GalloDaSballo
Total Solo HM: 13
Id: 130
League: ETH
Rank: 23/75
Findings: 3
Award: $308.11
🌟 Selected for report: 0
🚀 Solo Findings: 0
🌟 Selected for report: MiloTruck
Also found by: 0x52, Dravee, IllIllI, MaratCerby, WatchPug, unforgiven
https://github.com/code-423n4/2022-05-velodrome/blob/7fda97c570b758bbfa7dd6724a336c43d4041740/contracts/contracts/Bribe.sol#L50 https://github.com/code-423n4/2022-05-velodrome/blob/7fda97c570b758bbfa7dd6724a336c43d4041740/contracts/contracts/Bribe.sol#L87 https://github.com/code-423n4/2022-05-velodrome/blob/7fda97c570b758bbfa7dd6724a336c43d4041740/contracts/contracts/Gauge.sol#L359 https://github.com/code-423n4/2022-05-velodrome/blob/7fda97c570b758bbfa7dd6724a336c43d4041740/contracts/contracts/Gauge.sol#L516 https://github.com/code-423n4/2022-05-velodrome/blob/7fda97c570b758bbfa7dd6724a336c43d4041740/contracts/contracts/Gauge.sol#L561 https://github.com/code-423n4/2022-05-velodrome/blob/7fda97c570b758bbfa7dd6724a336c43d4041740/contracts/contracts/Gauge.sol#L604 https://github.com/code-423n4/2022-05-velodrome/blob/7fda97c570b758bbfa7dd6724a336c43d4041740/contracts/contracts/Gauge.sol#L610 https://github.com/code-423n4/2022-05-velodrome/blob/7fda97c570b758bbfa7dd6724a336c43d4041740/contracts/contracts/Pair.sol#L152 https://github.com/code-423n4/2022-05-velodrome/blob/7fda97c570b758bbfa7dd6724a336c43d4041740/contracts/contracts/Pair.sol#L162 https://github.com/code-423n4/2022-05-velodrome/blob/7fda97c570b758bbfa7dd6724a336c43d4041740/contracts/contracts/Pair.sol#L324 https://github.com/code-423n4/2022-05-velodrome/blob/7fda97c570b758bbfa7dd6724a336c43d4041740/contracts/contracts/Pair.sol#L325 https://github.com/code-423n4/2022-05-velodrome/blob/7fda97c570b758bbfa7dd6724a336c43d4041740/contracts/contracts/Pair.sol#L345 https://github.com/code-423n4/2022-05-velodrome/blob/7fda97c570b758bbfa7dd6724a336c43d4041740/contracts/contracts/Pair.sol#L346 https://github.com/code-423n4/2022-05-velodrome/blob/7fda97c570b758bbfa7dd6724a336c43d4041740/contracts/contracts/Pair.sol#L371 https://github.com/code-423n4/2022-05-velodrome/blob/7fda97c570b758bbfa7dd6724a336c43d4041740/contracts/contracts/Pair.sol#L372 https://github.com/code-423n4/2022-05-velodrome/blob/7fda97c570b758bbfa7dd6724a336c43d4041740/contracts/contracts/PairFees.sol#L29 https://github.com/code-423n4/2022-05-velodrome/blob/7fda97c570b758bbfa7dd6724a336c43d4041740/contracts/contracts/PairFees.sol#L30 https://github.com/code-423n4/2022-05-velodrome/blob/7fda97c570b758bbfa7dd6724a336c43d4041740/contracts/contracts/Router.sol#L201 https://github.com/code-423n4/2022-05-velodrome/blob/7fda97c570b758bbfa7dd6724a336c43d4041740/contracts/contracts/Router.sol#L202 https://github.com/code-423n4/2022-05-velodrome/blob/7fda97c570b758bbfa7dd6724a336c43d4041740/contracts/contracts/Router.sol#L225 https://github.com/code-423n4/2022-05-velodrome/blob/7fda97c570b758bbfa7dd6724a336c43d4041740/contracts/contracts/Router.sol#L272 https://github.com/code-423n4/2022-05-velodrome/blob/7fda97c570b758bbfa7dd6724a336c43d4041740/contracts/contracts/Router.sol#L342 https://github.com/code-423n4/2022-05-velodrome/blob/7fda97c570b758bbfa7dd6724a336c43d4041740/contracts/contracts/Router.sol#L357 https://github.com/code-423n4/2022-05-velodrome/blob/7fda97c570b758bbfa7dd6724a336c43d4041740/contracts/contracts/Router.sol#L385 https://github.com/code-423n4/2022-05-velodrome/blob/7fda97c570b758bbfa7dd6724a336c43d4041740/contracts/contracts/Router.sol#L399 https://github.com/code-423n4/2022-05-velodrome/blob/7fda97c570b758bbfa7dd6724a336c43d4041740/contracts/contracts/Voter.sol#L257 https://github.com/code-423n4/2022-05-velodrome/blob/7fda97c570b758bbfa7dd6724a336c43d4041740/contracts/contracts/VotingEscrow.sol#L358
Every time transferFrom or transfer function in ERC20 standard is called there is a possibility that underlying smart contract did not transfer the exact amount entered. It is required to find out contract balance increase/decrease after the transfer to allow fee-on-transfer tokens or forbid non-standard tokens. This pattern also prevents from re-entrancy attack vector.
POC (re-entrancy for fee-on-transfer tokens):
There are several possible scenarios to prevent that.
Recommended example code:
enum FeeOnTransferStatuses{ NOT_INITIALIZED, HAS_FEE_ON_TRANSFER, DOES_NOT_HAVE_FEE_ON_TRANSFER } mapping(IERC20 => FeeOnTransferStatuses) doesThisContractHaveFeeOnTransfer; error FeeOnTransferTokensAreForbidden(); ... function deposit(IERC20 token, address from, uint256 amount) public { // reverting for fee-on-transfer tokens if (doesThisContractHaveFeeOnTransfer[token] == FeeOnTransferStatuses.HAS_FEE_ON_TRANSFER) { revert FeeOnTransferTokensAreForbidden(); } // NOT_INITIALIZED is the default value == 0 if (doesThisContractHaveFeeOnTransfer[token] == FeeOnTransferStatuses.NOT_INITIALIZED) { uint256 balanceBefore = token.balanceOf(address(this)); // remembering asset balance before the transfer token.safeTransferFrom(from, address(this), amount); uint256 balanceAfter = token.balanceOf(address(this)); // making sure exactly amount was transferred if (balanceAfter != balanceBefore + amount) { revert FeeOnTransferTokensAreForbidden(); } // IMPORTANT! if you allow fee-on-transfer tokens make sure to update amount with the actual balance increase/decrease // or make sure balanceAfter - balanceBefore == amount using require // amount = balanceAfter - balanceBefore; // commented because we skip fee-on-transfer tokens above doesThisContractHaveFeeOnTransfer[token] = FeeOnTransferStatuses.DOES_NOT_HAVE_FEE_ON_TRANSFER; // making sure to not enter this if clause anymore for given token } else { // token does not have fee-on-transfer here token.safeTransferFrom(from, address(this), amount); } // no re-entrancy vector attack below here // amount is set to exactly how much contract balance was increased registerDeposit(from, amount); }
#0 - pooltypes
2022-06-13T15:54:44Z
Duplicate of #222
#1 - GalloDaSballo
2022-06-28T23:41:53Z
Dup of #222
🌟 Selected for report: IllIllI
Also found by: 0x1f8b, 0x52, 0xNazgul, 0xNineDec, AlleyCat, BouSalman, CertoraInc, Chom, Dravee, Funen, GimelSec, Hawkeye, MaratCerby, Nethermind, Picodes, RoiEvenHaim, SooYa, TerrierLover, WatchPug, _Adam, asutorufos, berndartmueller, c3phas, catchup, cccz, cryptphi, csanuragjain, delfin454000, djxploit, fatherOfBlocks, gzeon, hake, hansfriese, horsefacts, hyh, jayjonah8, minhquanym, oyc_109, p_crypt0, pauliax, robee, rotcivegaf, sach1r0, sashik_eth, simon135, sorrynotsorry, teddav, unforgiven, xiaoming90
101.277 USDC - $101.28
Please paste contents of this file. It exceeds the limit.
https://software.valar-solutions.com/Hosting/screenshots/2022-05-25_16-14-59.md
#0 - GalloDaSballo
2022-07-04T21:35:02Z
[1] IF nesting could be reduced by having early return/continue.
Affected code:
[2] Multiple divisions can degradate precision. Consider using single division instead.
Affected code:
[3] Token decimals is a constant parameter that never changes. Consider caching it.
Affected code:
[4] By default, function types and state variables/constants are internal, so the internal keyword can be omitted.
Affected code:
[5] Magic number, consider using named constant instead.
Affected code:
[6] Consider using "_" separate digit capacity i.e "100000" could be replaced to "100_000". This increases code readability.
Affected code:
[7] Consider using IERC20 type instead of address. Or IERC20[] type instead of address[].
Affected code:
[8] Uint8-256 / Int8-256 is assigned to zero by default, additional reassignment to zero is unnecessary.
Affected code:
[9] It is recommended to explicitly specify uint256 type instead of uint type for better readability.
Affected code:
#1 - GalloDaSballo
2022-07-04T21:41:10Z
Valid NC
Disagree because they are used for floor math
Finding is invalid
This is against best practices
Valid NC
Valid NC
Disagree as it's just opinion
Valid NC
What were you thinking to paste 675 links???
Very soulles report 4 NC
🌟 Selected for report: IllIllI
Also found by: 0x1f8b, 0x4non, 0xNazgul, 0xf15ers, 0xkatana, Chom, ControlCplusControlV, DavidGialdi, Deivitto, Dravee, ElKu, Fitraldys, Funen, GimelSec, MadWookie, MaratCerby, MiloTruck, Picodes, Randyyy, TerrierLover, TomJ, Tomio, UnusualTurtle, WatchPug, Waze, _Adam, asutorufos, c3phas, catchup, csanuragjain, delfin454000, djxploit, fatherOfBlocks, gzeon, hake, hansfriese, horsefacts, orion, oyc_109, pauliax, reassor, rfa, rotcivegaf, sach1r0, saian, sashik_eth, simon135, supernova, teddav, z3s
59.4006 USDC - $59.40
[1] Total supply will overflow earlier than balanceOf[_to] thats why you can save gas by using unchecked keyword here.
Affected code:
[2] balanceOf[dst] will underflow earlier than totalSupply thats why you can save gas by using unchecked keyword here.
Affected code:
[3] --ownerToNFTokenCount[_from]; would save 6 gas here.
Affected code:
[4] Using storage keyword would save ~500-2000 gas here.
Affected code:
[5] Consider using optimized for-loop and apply the following optimizations:
Affected code:
[6] Using x != 0 uses 6 less gas than x > 0. Consider changing all "greater than zero" comparisons to "not equal to zero".
Affected code:
[7] Using ++x uses 5 less gas than x++ (same applies to --x). Consider changing all "x++" operators to "++x" (same applies to --x).
Affected code:
[8] The power of 10 numbers such as "10**18" could be rendered as "1e18".
Affected code:
[9] Splitting && conditions into several require statements saves gas.
Affected code:
[10] As per 0.8.4 solidity version it supports new custom errors. Custom errors are reducing 38 gas if condition is met and 22 gas otherwise. Also reduces contract size and deployment costs.
Affected code:
#0 - pooltypes
2022-06-10T03:05:08Z
Duplicate of #69
#1 - GalloDaSballo
2022-07-03T22:09:06Z
Rating as 20 per improvement 80
20
I believe it's 5 gas for pre-increment
In Lack of POC I can only give 6 gas per variable (3 MSTORE, 3 MLOAD), please provide POC with your gas findings 84
Most of these have length inputed so am not going to give it those savings, valid pre-increment and unchecked, will give 25 per instance 1050
Not anymore for Solidity >= 0.8.12
5
Doesn't save gas
3 per instance 60
Disagree with the cookie cutter no POC math
Total Gas Saved 1304