Platform: Code4rena
Start Date: 28/11/2022
Pot Size: $192,500 USDC
Total HM: 33
Participants: 106
Period: 11 days
Judge: LSDan
Total Solo HM: 15
Id: 186
League: ETH
Rank: 49/106
Findings: 2
Award: $249.86
🌟 Selected for report: 0
🚀 Solo Findings: 0
🌟 Selected for report: IllIllI
Also found by: 0x4non, 0x52, 0xAgro, 0xNazgul, 0xSmartContract, 0xackermann, 9svR6w, Awesome, Aymen0909, B2, BRONZEDISC, Bnke0x0, Deekshith99, Deivitto, Diana, Dravee, HE1M, Jeiwan, Kaiziron, KingNFT, Lambda, Mukund, PaludoX0, RaymondFam, Rolezn, Sathish9098, Secureverse, SmartSek, __141345__, ahmedov, ayeslick, brgltd, cccz, ch0bu, chrisdior4, cryptonue, cryptostellar5, csanuragjain, datapunk, delfin454000, erictee, gz627, gzeon, helios, i_got_hacked, ignacio, imare, jadezti, jayphbee, joestakey, kankodu, ksk2345, ladboy233, martin, nadin, nicobevi, oyc_109, pashov, pavankv, pedr02b2, pzeus, rbserver, ronnyx2017, rvierdiiev, shark, unforgiven, xiaoming90, yjrwkk
103.9175 USDC - $103.92
Cross-Chain Replay attackStoring the block.chainid is not safe.
block.chainid,
block.number(block.number - updatedAt) <= config.expirationPeriod,
assetPriceMapEntry.updatedAt = block.number;
Other instances of this issue are :
block.timestampBlock timestamps have historically been used for a variety of applications, such as entropy for random numbers (see the Entropy Illusion for further details), locking funds for periods of time, and various state-changing conditional statements that are time-dependent. Miners have the ability to adjust timestamps slightly, which can prove to be dangerous if block timestamps are used incorrectly in smart contracts.
startTime: block.timestamp
assetPriceMapEntry.updatedTimestamp = block.timestamp;
Other instances of this issue are :
TODO commentsCode architecture, incentives, and error handling/reporting questions/issues should be resolved before deployment.
// TODO: support PToken
// TODO using bit shifting for the 2^96
Other instances of this issue are :
scientific notation (e.g. 1e18) rather than exponentiation (e.g. 10**18)((oracleData.token0Price * (10**18)) /
File: misc/UniswapV3OracleWrapper.sol (line 245)
TYPOS//@audit `tsatr` * @notice Function to tsatr auction on an ERC721 of a position if its ERC721 Health Factor drops below 1.
File: libraries/logic/AuctionLogic.sol (line 34)
//@audit `aggeregate` /// aggeregate prices which are not expired from different feeders, if number of valid/unexpired prices /// not enough, we do not aggeregate and just use previous price
File: contracts/misc/NFTFloorOracle.sol (line 53-54)
Other instances of this issue are :
aggeregateundate__gap[50] storage variable to allow for new storage variables in later versionsWhile some contracts may not currently be sub-classed, adding the variable now protects against forgetting to add it in the future.
contract WPunkGateway is ReentrancyGuard, IWPunkGateway, IERC721Receiver, OwnableUpgradeable
File: contracts/ui/WPunkGateway.sol (line 19-23)
contract NFTFloorOracle is Initializable, AccessControl, INFTFloorOracle {
File: contracts/misc/NFTFloorOracle.sol (line 55)
Other instances of this issue are :
INITIALIZE functions can be front-runThis function can be called by everyone. So an attacker may watch the mempool for this function and may frontrun it by supplying more gas.This may result in unintended behaviour.
function initialize( address _admin, address[] memory _feeders, address[] memory _assets ) public initializer {
File: contracts/misc/NFTFloorOracle.sol (line 97-101)
function initialize( IPool initializingPool, address underlyingAsset, IRewardController incentivesController, string calldata nTokenName, string calldata nTokenSymbol, bytes calldata params ) public virtual override initializer {
File: contracts/protocol/tokenization/NTokenApeStaking.sol (line 36-43)
Other instances of this issue are :
initializer modifier on constructorOpenZeppelin recommends that the initializer modifier be applied to constructors in order to avoid potential griefs, social engineering, or exploits. Ensure that the modifier is applied to the implementation contract. If the default constructor is currently being used, it should be changed to be an explicit one with the modifier applied.
contract WPunkGateway is ReentrancyGuard, IWPunkGateway, IERC721Receiver, OwnableUpgradeable
File: contracts/ui/WPunkGateway.sol (line 19-23)
contract NFTFloorOracle is Initializable, AccessControl, INFTFloorOracle {
File: contracts/misc/NFTFloorOracle.sol (line 55)
Other instances of this issue are :
public functions not called by the contract should be declared external insteadContracts are allowed to override their parents’ functions and change the visibility from external to public.
function initialize( address _admin, address[] memory _feeders, address[] memory _assets ) public initializer {
function collateralizedBalanceOf(address account) public
zero address check in constructorMissing checks for zero-addresses may lead to infunctional protocol, if the variable addresses are updated incorrectly.
punk = _punk; wpunk = _wpunk; pool = _pool;
File: contracts/ui/WPunkGateway.sol (line 48-50)
transferOwnership(owner);
File: protocol/configuration/PoolAddressesProvider.sol (line 47)
Other instances of this issue are :
garbage value in mapping for deleting thatIf there is a mapping data structure present inside struct, then deleting the struct doesn't delete the mapping. Instead one should use lock to lock that data structure from further use.
delete reservesData[asset];
File:contracts/protocol/libraries/logic/PoolLogic.sol (line 152)
delete assetPriceMap[_asset]; delete assetFeederMap[_asset];
File:contracts/misc/NFTFloorOracle.sol (line 302-303)
NatSpec is incomplete/// @audit Missing: `@return` /** * @notice Validates the health factor of a user. * @param reservesData The state of all the reserves * @param reservesList The addresses of all the active reserves * @param userConfig The state of the user for the specific reserve * @param user The user to validate health factor of * @param reservesCount The number of available reserves * @param oracle The price oracle */ function validateHealthFactor(
/// @audit Missing: `@param` /** * @notice Validates a transfer action. * @param reserve The reserve object */ function validateTransferERC721(
Other instances of this issue are :
@return@param@param@paramparam@param@param@param@return@return@param@param& return@param & return#0 - c4-judge
2023-01-25T16:13:04Z
dmvt marked the issue as grade-b
🌟 Selected for report: IllIllI
Also found by: B2, Deivitto, Dravee, PaludoX0, RaymondFam, Rolezn, Sathish9098, _Adam, ahmedov, c3phas, chrisdior4, cyberinn, rjs, saneryee
145.9382 USDC - $145.94
require() statements that use && saves gasInstead of using the && operator in a single require statement to check multiple conditions, I suggest using multiple require statements with 1 condition per require statement (saving 3 gas per &):
require( vars.collateralReserveActive && vars.principalReserveActive,
File: libraries/logic/ValidationLogic.sol (line 546-547)
require( !vars.collateralReservePaused && !vars.principalReservePaused,
File: protocol/libraries/logic/ValidationLogic.sol (line 550-551)
Other instances of the issue are:
internal functions only called once can be inlined to save gasNot inlining costs 20 to 40 gas because of two extra JUMP instructions and additional stack operations needed for function calls.
function getDomainSeparator() internal view returns (bytes32) {
File: logic/ValidationLogic.sol (line 1133)
function _delegateToPool(
File: protocol/libraries/logic/MarketplaceLogic.sol (line 376)
Other instances of the issue are:
When using elements that are smaller than 32 bytes, your contract’s gas usage may be higher. This is because the EVM operates on 32 bytes at a time. Therefore, if the element is smaller than that, the EVM must use more operations in order to reduce the size of the element from 32 bytes to the desired size.
uint16 referralCode
File: contracts/protocol/pool/PoolMarketplace.sol (line 75)
uint16 referralCode
File: contracts/protocol/pool/PoolMarketplace.sol (line 94)
Other instances of the issue are:
abi.encode() is less efficient than abi.encodePacked()abi.encode(
File: protocol/libraries/logic/ValidationLogic.sol (line 1124)
abi.encode(
File: contracts/protocol/libraries/logic/ValidationLogic.sol (line 1136)
returnsUsing both named returns and a return statement isn’t necessary. Removing one of those can improve code clarity:
return (collectionLtv, collectionLT);
File: protocol/libraries/logic/PoolLogic.sol (line 236)
return assetPriceMap[_asset].twap;
File: contracts/misc/NFTFloorOracle.sol (line 247)
Other instances of the issue are:
address mappings can be combined into a single mapping of address to a struct, where appropriate.Saves a storage slot for the mapping. Depending on the circumstances and sizes of types, can avoid a Gsset (20000 gas) per mapping combined. Reads and subsequent writes can also be cheaper when a function requires both values and they both fit in the same storage slot
463 mapping(address => DataTypes.ReserveData) storage reservesData, 465 mapping(address => DataTypes.UserConfigurationMap) storage usersConfig,
524 mapping(address => DataTypes.ReserveData) storage reservesData, 526 mapping(address => DataTypes.UserConfigurationMap) storage usersConfig,
Other instances of the issue are:
receive()Â functions that are not used, can be removed to save deployment gasreceive() external payable {}
File: contracts/protocol/tokenization/NTokenUniswapV3.sol (line 149)
elementary or custom data type instead of struct with only 1 memberstruct APEStakingParameter { uint256 unstakeIncentive; }
File: contracts/protocol/tokenization/NTokenUniswapV3.sol (line 26-28)
require functions in loop . It is better to break than to revertfor (uint256 index = 0; index < amount; index++) { // validate that the owner of the underlying asset is the NToken contract require(
File: libraries/logic/ValidationLogic.sol (line 131-133)
require(
File: protocol/libraries/logic/ValidationLogic.sol (line 140)
Other instances of the issue are:
#0 - c4-judge
2023-01-25T16:15:59Z
dmvt marked the issue as grade-b