Platform: Code4rena
Start Date: 14/06/2022
Pot Size: $100,000 USDC
Total HM: 26
Participants: 59
Period: 7 days
Judge: GalloDaSballo
Total Solo HM: 9
Id: 133
League: ETH
Rank: 27/59
Findings: 4
Award: $731.33
🌟 Selected for report: 0
🚀 Solo Findings: 0
1207.2233 CANTO - $194.97
194.9666 USDC - $194.97
In WETH.sol 2nd approve function can be used to update allowance of any user, it can be used to steal users balance
function approve(address owner, address spender) external returns(bool) { _approve(owner, spender, _balanceOf[owner]); return true; } function _approve( address owner, address spender, uint256 amount ) internal { require(owner != address(0), "ERC20: approve from the zero address"); require(spender != address(0), "ERC20: approve to the zero address"); _allowance[owner][spender] = amount; emit Approval(owner, spender, amount); }
Manual review
The 2nd approve function can be removed, allowance can be updated from the 1st approve function
#0 - nivasan1
2022-06-24T03:32:40Z
duplicate of #19
#1 - GalloDaSballo
2022-08-04T17:59:28Z
Dup if #19
🌟 Selected for report: joestakey
Also found by: 0x1f8b, 0x29A, 0x52, 0xDjango, 0xNazgul, 0xf15ers, 0xmint, Bronicle, Dravee, Funen, JMukesh, Limbooo, MadWookie, Picodes, Ruhum, TerrierLover, TomJ, Tutturu, WatchPug, Waze, _Adam, asutorufos, c3phas, catchup, cccz, codexploder, cryptphi, csanuragjain, defsec, fatherOfBlocks, gzeon, hake, hansfriese, hyh, ignacio, k, nxrblsrpr, oyc_109, robee, sach1r0, saian, simon135, technicallyty, zzzitron
72.5532 USDC - $72.55
687.9945 CANTO - $111.11
Contract does not contain comments
Add comments for the function in the contracts
In lending market WETH.sol some lines of code are commented out
The commented out code can be removed
Contract contains open todos, it can be fixed and removed
uint constant periodSize = 1800;
require statements doesnt have revert messages, if condition fails the transaction would revert silently with any error messages
require(amountADesired >= amountAMin); require(amountBDesired >= amountBMin);
Add revert messages
#0 - GalloDaSballo
2022-08-03T23:32:30Z
NC
NC
NC
R
NC
1R 4NC
🌟 Selected for report: _Adam
Also found by: 0v3rf10w, 0x1f8b, 0x29A, 0xKitsune, 0xNazgul, 0xf15ers, 0xkatana, 0xmint, Chom, Dravee, Fitraldys, Funen, JC, Limbooo, MadWookie, Picodes, Ruhum, TerrierLover, TomJ, Tomio, Waze, ak1, c3phas, catchup, defsec, fatherOfBlocks, gzeon, hake, hansfriese, joestakey, k, oyc_109, rfa, robee, sach1r0, saian, simon135, ynnad
396.9199 CANTO - $64.10
93.6306 USDC - $93.63
When variables are created it contains default values, explicitly initialising with default values (0, address(0), false, ) is not needed
dripped = 0;
uint public totalSupply = 0;
isPaused = false;
Variable that are initialised during construction and not updated later can be converted to immutable to save gas
dripStart = block.number; dripRate = dripRate_; token = token_; target = target_;
address private admin;
Values used once can be directly added to the expression instead of storing in a temporary variable
token, dripRate_, dripStart_, blockNumber_, target_, drippedNext_ in
EIP20Interface token_ = token; uint reservoirBalance_ = token_.balanceOf(address(this)); // TODO: Verify this is a static call uint dripRate_ = dripRate; uint dripStart_ = dripStart; uint dripped_ = dripped; address target_ = target; uint blockNumber_ = block.number; // Next, calculate intermediate values uint dripTotal_ = mul(dripRate_, blockNumber_ - dripStart_, "dripTotal overflow"); uint deltaDrip_ = sub(dripTotal_, dripped_, "deltaDrip underflow"); uint toDrip_ = min(reservoirBalance_, deltaDrip_); uint drippedNext_ = add(dripped_, toDrip_, "tautological"); // Finally, write new `dripped` value and transfer tokens to target dripped = drippedNext_; token_.transfer(target_, toDrip_);
name and symbol in
string public name; string public symbol;
Since GovernorBravoDelegate.sol uses solidity version 0.8, when addition overflows the execution reverts and require statement is not executed
uint c = a + b; require(c >= a, "addition overflow");
unchecked can be added to the addition statement
unchecked { c = a + b; }
msg.sender != address(0)In lender market GovernorBravoDelegate.sol msg.sender is checked for address(0)
require(msg.sender == pendingAdmin && msg.sender != address(0), "GovernorBravo:_acceptAdmin: pending admin only");
address(0) check can be removed
> 0 can be replaced with != 0!= 0 costs less gas compared to > 0 for unsigned integers in require statements with optimizer enabled
require(amountA > 0, 'BaseV1Router: INSUFFICIENT_AMOUNT');
require(reserveA > 0 && reserveB > 0, 'BaseV1Router: INSUFFICIENT_LIQUIDITY');
https://github.com/Plex-Engineer/stableswap/blob/489d010eb99a0885139b2d5ed5a2d826838cc5f9/contracts/BaseV1-periphery.sol#L456 https://github.com/Plex-Engineer/stableswap/blob/489d010eb99a0885139b2d5ed5a2d826838cc5f9/contracts/BaseV1-periphery.sol#L463
Revert string greater than 32 bytes will increase deployment gas as well as runtime gas when the condition is met
L138 : require(liquidity > 0, 'UniswapV2: INSUFFICIENT_LIQUIDITY_MINTED'); L159 : require(amount0 > 0 && amount1 > 0, 'UniswapV2: INSUFFICIENT_LIQUIDITY_BURNED'); L173 require(amount0Out > 0 || amount1Out > 0, 'UniswapV2: INSUFFICIENT_OUTPUT_AMOUNT'); L175: require(amount0Out < _reserve0 && amount1Out < _reserve1, 'UniswapV2: INSUFFICIENT_LIQUIDITY'); L191: require(amount0In > 0 || amount1In > 0, 'UniswapV2: INSUFFICIENT_INPUT_AMOUNT');
L52 : require(amountBOptimal >= amountBMin, 'UniswapV2Router: INSUFFICIENT_B_AMOUNT'); L57 : require(amountAOptimal >= amountAMin, 'UniswapV2Router: INSUFFICIENT_A_AMOUNT'); L118 : require(amountA >= amountAMin, 'UniswapV2Router: INSUFFICIENT_A_AMOUNT'); L119 : require(amountB >= amountBMin, 'UniswapV2Router: INSUFFICIENT_B_AMOUNT'); L233 : require(amounts[amounts.length - 1] >= amountOutMin, 'UniswapV2Router: INSUFFICIENT_OUTPUT_AMOUNT'); L247 : require(amounts[0] <= amountInMax, 'UniswapV2Router: EXCESSIVE_INPUT_AMOUNT'); L263 : require(amounts[amounts.length - 1] >= amountOutMin, 'UniswapV2Router: INSUFFICIENT_OUTPUT_AMOUNT'); L277 : require(amounts[0] <= amountInMax, 'UniswapV2Router: EXCESSIVE_INPUT_AMOUNT'); L294 : require(amounts[amounts.length - 1] >= amountOutMin, 'UniswapV2Router: INSUFFICIENT_OUTPUT_AMOUNT'); L312 : require(amounts[0] <= msg.value, 'UniswapV2Router: EXCESSIVE_INPUT_AMOUNT'); L354 : require( IERC20Uniswap(path[path.length - 1]).balanceOf(to).sub(balanceBefore) >= amountOutMin, 'UniswapV2Router: INSUFFICIENT_OUTPUT_AMOUNT' ); L377 : require( IERC20Uniswap(path[path.length - 1]).balanceOf(to).sub(balanceBefore) >= amountOutMin, 'UniswapV2Router: INSUFFICIENT_OUTPUT_AMOUNT' ); L398 : require(amountOut >= amountOutMin, 'UniswapV2Router: INSUFFICIENT_OUTPUT_AMOUNT');
L46 : require(granularity_ > 1, 'SlidingWindowOracle: GRANULARITY'); L49 : require( (periodSize = windowSize_ / granularity_) * granularity_ == windowSize_, 'SlidingWindowOracle: WINDOW_NOT_EVENLY_DIVISIBLE' ); L133 : require(timeElapsed <= windowSize, 'SlidingWindowOracle: MISSING_HISTORICAL_OBSERVATION'); L135 : require(timeElapsed >= windowSize - periodSize * 2, 'SlidingWindowOracle: UNEXPECTED_TIME_ELAPSED');
L86 : require(tokenA != tokenB, 'BaseV1Router: IDENTICAL_ADDRESSES'); L104 : require(amountA > 0, 'BaseV1Router: INSUFFICIENT_AMOUNT'); L105 : require(reserveA > 0 && reserveB > 0, 'BaseV1Router: INSUFFICIENT_LIQUIDITY'); L295 : require(amountA >= amountAMin, 'BaseV1Router: INSUFFICIENT_A_AMOUNT'); L296 : require(amountB >= amountBMin, 'BaseV1Router: INSUFFICIENT_B_AMOUNT'); L387 : require(amounts[amounts.length - 1] >= amountOutMin, 'BaseV1Router: INSUFFICIENT_OUTPUT_AMOUNT'); L402 : require(amounts[amounts.length - 1] >= amountOutMin, 'BaseV1Router: INSUFFICIENT_OUTPUT_AMOUNT'); L417 : require(amounts[amounts.length - 1] >= amountOutMin, 'BaseV1Router: INSUFFICIENT_OUTPUT_AMOUNT'); L430 : require(amounts[amounts.length - 1] >= amountOutMin, 'BaseV1Router: INSUFFICIENT_OUTPUT_AMOUNT'); L452 : require(success, 'TransferHelper: ETH_TRANSFER_FAILED');
L87 : require(state(proposalId) == ProposalState.Queued, "GovernorBravo::execute: proposal can only be executed if it is queued"); L78 : require(!timelock.queuedTransactions(keccak256(abi.encode(target, value, signature, data, eta))), "GovernorBravo::queueOrRevertInternal: identical proposal action already queued at eta"); L25 : require(address(timelock) == address(0), "GovernorBravo::initialize: can only initialize once"); L26 : require(msg.sender == admin, "GovernorBravo::initialize: admin only"); L27 : require(timelock_ != address(0), "GovernorBravo::initialize: invalid timelock address");
require(unigovProposal.targets.length == unigovProposal.values.length && unigovProposal.targets.length == unigovProposal.signatures.length && unigovProposal.targets.length == unigovProposal.calldatas.length, "GovernorBravo::propose: proposal function information arity mismatch"); require(unigovProposal.targets.length != 0, "GovernorBravo::propose: must provide actions"); require(unigovProposal.targets.length <= proposalMaxOperations, "GovernorBravo::propose: too many actions");
L17 : require(msg.sender == admin, "AccountantDelegate::initialize: only admin can call this function"); L18 : require(noteAddress_ != address(0), "AccountantDelegate::initialize: note Address invalid"); L29 : require(note.balanceOf(msg.sender) == note._initialSupply(), "AccountantDelegate::initiatlize: Accountant has not received payment"); L48 : require(msg.sender == address(cnote), "AccountantDelegate::supplyMarket: Only the CNote contract can supply market"); L60 : require(msg.sender == address(cnote), "AccountantDelegate::redeemMarket: Only the CNote contract can redeem market"); L83 : require(cNoteConverted >= noteDifferential, "Note Loaned to LendingMarket must increase in value");
++i is more efficient than i++pre-increment is more efficient than post-increment for unsigned integers, and unchecked can be added to avoid the default underflow/overflow protections introduced in 0.8 to save more gas
for (uint i; i < path.length - 1; i++) {
for (uint i; i < path.length - 1; i++) {
for (uint i = pairObservations[pair].length; i < granularity; i++) {
for (uint i = 0; i < _prices.length; i++) {
Array length can be cached and used in the for-loops instead of querying in every iteration
for (uint i; i < path.length - 1; i++) {
for (uint i; i < path.length - 1; i++) {
for (uint i = 0; i < _prices.length; i++) {
for (uint i = 0; i < routes.length; i++) {
for (uint i = 0; i < proposal.targets.length; i++) {
for (uint i = 0; i < affectedUsers.length; ++i) {
for (uint i = 0; i < allMarkets.length; i ++) {
#0 - GalloDaSballo
2022-08-04T00:42:29Z
10.5k for immutables
Rest is less than 1k