Platform: Code4rena
Start Date: 03/05/2022
Pot Size: $50,000 USDC
Total HM: 4
Participants: 46
Period: 5 days
Judge: gzeon
Total Solo HM: 2
Id: 117
League: ETH
Rank: 22/46
Findings: 2
Award: $178.36
🌟 Selected for report: 0
🚀 Solo Findings: 0
🌟 Selected for report: BowTiedWardens
Also found by: 0x1337, 0x1f8b, 0x4non, 0xDjango, David_, Funen, GimelSec, IllIllI, Picodes, TerrierLover, WatchPug, bobi, cryptphi, csanuragjain, delfin454000, dirk_y, ellahi, fatherOfBlocks, hyh, ilan, jayjonah8, kebabsec, leastwood, oyc_109, robee, samruna, simon135, sorrynotsorry, throttle
93.5794 USDC - $93.58
Typos
* @param addAmount The amount fo underlying token to add as reserves
Change fo to to
// Checck for overflow.
Change Checck to Check
Issue: Missing require message
Explanation: Require message should be included to enable users to understand reason for failure
function getCashPrior() internal view returns (uint) { (MathError err, uint startingBalance) = subUInt(address(this).balance, msg.value); require(err == MathError.NO_ERROR); return startingBalance; }
Recommendation: Add a brief (<= 32 char) message to explain require failure
🌟 Selected for report: BowTiedWardens
Also found by: 0v3rf10w, 0x1f8b, 0x4non, 0xDjango, 0xNazgul, 0xkatana, Cityscape, Fitraldys, Funen, GimelSec, IllIllI, MaratCerby, Picodes, TerrierLover, Tomio, delfin454000, ellahi, fatherOfBlocks, hansfriese, ilan, joestakey, oyc_109, rfa, robee, samruna, simon135, slywaters, throttle
84.784 USDC - $84.78
Issue: Require message is to long
Explanation: The messages below can be shortened to 32 characters or fewer (as shown) to save gas
require(buyPunkSuccess, "CNFT: Calling buyPunk was unsuccessful");
Change message to CNFT: buyPunk call unsuccessful
require(borrower != liquidator, "CNFT: Liquidator cannot be borrower");
Change message to CNFT: Liquidator cannot borrow
require(borrowBalance >= repayAmount, "Can not repay more than the total borrow");
Change message to Cannot repay > than total borrow
require(cNftCollateral == address(nftMarket), "cNFT is from the wrong comptroller");
Change message to cNFT is from wrong comptroller
require(msg.sender == admin, "only admin can set borrow cap guardian");
Change message to admin sets borrow cap guardian
require(address(nftMarket) == address(0), "nft collateral already initialized");
Change message to nft coll already initialized
require(msg.sender == admin, "only admin may initialize the market");
Change message to only admin may initialize market
require(err == uint(Error.NO_ERROR), "setting interest rate model failed");
Change message to interest rate model set failed
require(seizeTokens == 0, "LIQUIDATE_SEIZE_INCORRECT_NUM_NFTS");
Change message to LIQUIDATE_SEIZE_WRONG_NUM_NFTS
The same require message occurs in both lines referenced below: https://github.com/bunkerfinance/bunker-protocol/blob/752126094691e7457d08fc62a6a5006df59bd2fe/contracts/Comptroller.sol#L995
require(markets[address(cToken)].isListed, "cannot pause a market that is not listed");
Change message to can't pause a market not listed
The same require message occurs in both lines referenced below:
require(length > 0, "UniswapV2PriceOracle: No observations.");
Change message to UniswapV2PriceOracle: No obs.
Issue: Require message is to long but may be difficult to shorten
Explanation: The long require messages below might be difficult to cut back while preserving their chosen format and/or meaning:
require(address(token) != underlying, "CErc20::sweepToken: can not sweep underlying token");
require(msg.sender == admin, "only the admin may set the comp-like delegate");
require(_underlying != address(0), "CNFT: Asset should not be address(0)");
require(ComptrollerInterface(_comptroller).isComptroller(), "_comptroller is not a Comptroller contract");
require(buyPunkSuccess, "CNFT: Calling buyPunk was unsuccessful");
require(transferPunkSuccess, "CNFT: Calling transferPunk was unsuccessful");
require(msg.sender == underlying, "CNFT: This contract can only receive the underlying NFT");
require(operator == address(this), "CNFT: Only the CNFT contract can be the operator");
require(to != underlying, "CNFT: Cannot make an arbitrary call to underlying NFT");
require(oErr == 0, "exitMarket: getAccountSnapshot failed"); // semi-opaque error code
require(msg.sender == admin || msg.sender == borrowCapGuardian, "only admin or borrow cap guardian can set borrow caps");
require(msg.sender == unitroller.admin(), "only unitroller admin can change brains");
require(address(cNft) != address(0), "cannot initialize nft market to the 0 address");
require(initialExchangeRateMantissa > 0, "initial exchange rate must be greater than zero.");
require(err == MathError.NO_ERROR, "borrowBalanceStored: borrowBalanceStoredInternal failed");
require(err == MathError.NO_ERROR, "exchangeRateStored: exchangeRateStoredInternal failed");
require(vars.mathErr == MathError.NO_ERROR, "MINT_NEW_TOTAL_SUPPLY_CALCULATION_FAILED");
require(vars.mathErr == MathError.NO_ERROR, "MINT_NEW_ACCOUNT_BALANCE_CALCULATION_FAILED");
require(redeemTokensIn == 0 || redeemAmountIn == 0, "one of redeemTokensIn or redeemAmountIn must be zero");
require(vars.mathErr == MathError.NO_ERROR, "REPAY_BORROW_NEW_ACCOUNT_BORROW_BALANCE_CALCULATION_FAILED");
require(vars.mathErr == MathError.NO_ERROR, "REPAY_BORROW_NEW_TOTAL_BALANCE_CALCULATION_FAILED");
require(totalReservesNew <= totalReserves, "reduce reserves unexpected underflow");
require(totalReservesNew <= totalReserves, "reduce reserves unexpected underflow");
"CNftPriceOracle: Cannot overwrite existing address mappings."
"CNftPriceOracle: No NFTX token for cNFT."
The long require message below appears in the following two locations:
require(amountSeizeError == uint(Error.NO_ERROR), "LIQUIDATE_COMPTROLLER_CALCULATE_AMOUNT_SEIZE_FAILED");
The long require message below appears in the following two locations:
require(amounts[i] == 1, "CNFT: Amounts must be all 1s for non-ERC1155s.");
The long require message below appears in the following two locations:
"UniswapV2PriceOracle: Bad TWAP time."
The long require message below appears in the following two locations:
require(length > 1, "UniswapV2PriceOracle: Only one observation.");
The long require message below appears in the following three locations:
"UniswapV2PriceOracle: Division by zero."
The long require message below appears in the following four locations:
require(msg.sender == pauseGuardian || msg.sender == admin, "only pause guardian and admin can pause");
Issue: Should use != 0 instead of > 0 in a require statement if variable is an unsigned integer (uint)
Explanation: != 0 should be used where possible since > 0 costs more gas
require(initialExchangeRateMantissa > 0, "initial exchange rate must be greater than zero.");
Change initialExchangeRateMantissa > 0 to require(initialExchangeRateMantissa != 0
require( reserve0 > 0, "UniswapV2PriceOracle: Division by zero." );
Change reserve0 > 0 to reserve0 != 0
require( reserve1 > 0, "UniswapV2PriceOracle: Division by zero." );
Change reserve1 > 0 to reserve1 != 0
Identical requires appear in both lines below:
require(length > 0, "UniswapV2PriceOracle: No observations.");
Change length > 0 to length != 0 in both cases
There are two opportunities to save gas in the require below:
require message to 32 characters or fewer!= 0 instead of > 0 because > 0 costs more gas and initialExchangeRateMantissa cannot be less than zerorequire(initialExchangeRateMantissa > 0, "initial exchange rate must be greater than zero.");
Change to:
require(initialExchangeRateMantissa != 0, "initial exchange rate must be >0");
Issue: Use of '&&' within a require function
Explanation: Dividing the require into separate require messages instead of using '&&' will save gas
require(checkSuccess && nftOwner == msg.sender, "Not the NFT owner");
Recommendation:
require(checkSuccess, "Not the NFT owner"); require(nftOwner == msg.sender, "Not the NFT owner");
require(numMarkets != 0 && numMarkets == numBorrowCaps, "invalid input");
Recommendation:
require(numMarkets != 0, "invalid input"); require(numMarkets == numBorrowCaps, "invalid input");
require( cNfts.length > 0 && cNfts.length == nftxTokens.length, "CNftPriceOracle: `cNfts` and `nftxTokens` must have nonzero, equal lengths." );
Recommendation:
require(cNfts.length > 0, "CNftPriceOracle: `cNfts` must have nonzero lengths"); require(cNfts.length == nftxTokens.length, "CNftPriceOracle: `cNfts` and `nftxTokens` must have equal lengths");
There are two opportunities to save gas in the require below:
require message to 32 characters or fewerrequire into two separate requires instead of using '&&'require(accrualBlockNumber == 0 && borrowIndex == 0, "market may only be initialized once");
Recommendation:
require(accrualBlockNumber == 0, "accrualBlockNumber must equal 0"); require(borrowIndex == 0, "mkt may only be initialized once");
There are two opportunities to save gas in the require below:
!= 0 instead of > 0 because > 0 costs more gas and reserve0 and reserve1 cannot be < zero (since they are units)require into two separate requires instead of using '&&'require( reserve0 > 0 && reserve1 > 0, "UniswapV2PriceOracle: Division by zero." );
Recommendation:
require(reserve0 != 0, "UniswapV2PriceOracle: Division by zero."); require(reserve1 != 0, "UniswapV2PriceOracle: Division by zero.");
Issue: Variables should not be initialized to their default values
Explanation: Initializing uint variables to their default value of 0 is unnecessary and costs gas
uint256 totalAmount is initialized to zero three times in CNft.sol:
uint256 totalAmount = 0;
Recommendation:
uint256 totalAmount;
The following also contain initializations of variables to their default values:
uint startingAllowance = 0;
Recommendation:
uint startingAllowance;
uint256 numberUpdated = 0;
Recommendation:
uint256 numberUpdated;