bunker.finance contest - oyc_109's results

Lines of code

https://github.com/bunkerfinance/bunker-protocol/blob/752126094691e7457d08fc62a6a5006df59bd2fe/contracts/PriceOracleImplementation.sol#L29

Vulnerability details

description

When using Chainlink Price feeds it is important to ensure the price feed data was updated recently. While getting started with chainlink requires just one line of code, it is best to add additional checks for in production environments.

findings

PriceOracleImplementation.sol
29: int256 usdcPrice = ChainlinkFeed(0x986b5E1e1755e3C2440e960477f25201B0a8bbD4).latestAnswer();

should change to

    (uint80 roundID, int256 usdcPrice, , uint256 timestamp, uint80 answeredInRound) = ChainlinkFeed(0x986b5E1e1755e3C2440e960477f25201B0a8bbD4).latestRoundData();
    require(usdcPrice > 0, "ChainLink: USDC price <= 0");
    require(answeredInRound >= roundID, "ChainLink: Stale USDC price");
    require(timestamp > 0, "ChainLink: USDC round not complete");

Unspecific Compiler Version Pragma

description

Avoid floating pragmas for non-library contracts.

While floating pragmas make sense for libraries to allow them to be included with multiple different versions of applications, it may be a security risk for application implementations.

A known vulnerable compiler version may accidentally be selected or security tools might fall-back to an older compiler version ending up checking a different EVM compilation that is ultimately deployed on the blockchain.

It is recommended to pin to a concrete compiler version.

findings

CNft.sol
2: pragma solidity ^0.8.0;

do not cache variable used only once

description

for variables only used once, changing it to inline saves gas

findings

File: CNft.sol
44: uint mintAllowedResult = ComptrollerInterface(comptroller).mintAllowed(address(this), msg.sender, 0);
131: uint redeemAllowedResult = ComptrollerInterface(comptroller).redeemAllowed(address(this), msg.sender, totalAmount);
181: uint transferAllowedResult = ComptrollerInterface(comptroller).transferAllowed(address(this), from, to, vars.totalAmount);

File: CErc20.sol
137: uint256 balance = token.balanceOf(address(this));

Long Revert Strings

description

Shortening revert strings to fit in 32 bytes will decrease gas costs for deployment and gas costs when the revert condition has been met.

If the contract(s) in scope allow using Solidity >=0.8.4, consider using Custom Errors as they are more gas efficient while allowing developers to describe the error in detail using NatSpec.

findings

File: CNft.sol
52: require(amounts[i] == 1, "CNFT: Amounts must be all 1s for non-ERC1155s.");
100: require(seizeAmounts[i] == 1, "CNFT: Amounts must be all 1s for non-ERC1155s.");
124: require(amounts[i] == 1, "CNFT: Amounts must be all 1s for non-ERC1155s.");
204: revert("CNFT: Use safeBatchTransferFrom instead");
208: require(msg.sender == underlying, "CNFT: This contract can only receive the underlying NFT");
209: require(operator == address(this), "CNFT: Only the CNFT contract can be the operator");
279: require(to != underlying, "CNFT: Cannot make an arbitrary call to underlying NFT");

File: CToken.sol
32:         require(msg.sender == admin, "only admin may initialize the market");
33:         require(accrualBlockNumber == 0 && borrowIndex == 0, "market may only be initialized once");
37: require(initialExchangeRateMantissa > 0, "initial exchange rate must be greater than zero.");

File: UniswapV2PriceOracle.sol
68: "UniswapV2PriceOracle: Division by zero."
92: "UniswapV2PriceOracle: Division by zero."
116: "UniswapV2PriceOracle: Division by zero."
128: require(length > 0, "UniswapV2PriceOracle: No observations.");
137: "UniswapV2PriceOracle: Bad TWAP time."
149: require(length > 0, "UniswapV2PriceOracle: No observations.");
152: require(length > 1, "UniswapV2PriceOracle: Only one observation.");
158: "UniswapV2PriceOracle: Bad TWAP time."

File: CNftPriceOracle.sol
64: "CNftPriceOracle: `cNfts` and `nftxTokens` must have nonzero, equal lengths."
70: "CNftPriceOracle: Cannot overwrite existing address mappings."
90: "CNftPriceOracle: No NFTX token for cNFT."

Don't Initialize Variables with Default Value

description

Uninitialized variables are assigned with the types default value.

Explicitly initializing a variable with it's default value costs unnecesary gas.

findings

CNft.sol
119: uint256 totalAmount = 0;

require statements should be checked first

description

require statements can be placed earlier to reduce gas usage on revert ie move require to the top of the function if possible

findings

initialExchangeRateMantissa should be changed to initialExchangeRateMantissa_ and the require statement should be moved up before line 36

File: CToken.sol
37: require(initialExchangeRateMantissa > 0, "initial exchange rate must be greater than zero.");