It is recommended to add zero address checks for listed parameters.
3. Missing events
Risk
Low
Impact
Multiple contracts are not implementing events for critical functions. Lack of events makes it difficult for off-chain applications to monitor the protocol.
It is recommended to add missing events to listed functions.
4. Critical address change
Risk
Low
Impact
Changing critical addresses such as ownership should be a two-step process where the first transaction (from the old/current address) registers the new address (i.e. grants ownership) and the second transaction (from the new address) replaces the old address with the new one. This gives an opportunity to recover from incorrect addresses mistakenly used in the first step. If not, contract functionality might become inaccessible.
It is recommended to implement two-step process for changing critical addresses.
5. Use scientific notation
Risk
Non-Critical
Impact
Defining big numbers that consists of many digits or performing additional math calculations might lead to accidential errors and mistakes.
Proof of Concept
core/InfinityExchange.sol:1161: uint256 PRECISION = 10**4; // precision for division; similar to bps
core/InfinityOrderBookComplication.sol:338: uint256 PRECISION = 10**4; // precision for division; similar to bp
staking/InfinityStaker.sol:237: (userstakedAmounts[user][Duration.TWELVE_MONTHS].amount * 4)) / (10**18);
Tools Used
Manual Review / VSCode
Recommended Mitigation Steps
It is recommended to use scientific notation.
6. Missing indexing for events
Risk
Non-Critical
Impact
Events should index addresses which helps off-chain applications in monitoring the protocol.
Proof of Concept
core/InfinityExchange.sol:80: event CancelAllOrders(address user, uint256 newMinNonce);
core/InfinityExchange.sol:81: event CancelMultipleOrders(address user, uint256[] orderNonces);
core/InfinityExchange.sol:82: event NewWethTransferGasUnits(uint32 wethTransferGasUnits);
core/InfinityExchange.sol:83: event NewProtocolFee(uint16 protocolFee);
---
core/InfinityExchange.sol:85: event MatchOrderFulfilled(
bytes32 sellOrderHash,
bytes32 buyOrderHash,
address seller,
address buyer,
address complication, // address of the complication that defines the execution
address currency, // token address of the transacting currency
uint256 amount // amount spent on the order
);
---
core/InfinityExchange.sol:95: event TakeOrderFulfilled(
bytes32 orderHash,
address seller,
address buyer,
address complication, // address of the complication that defines the execution
address currency, // token address of the transacting currency
uint256 amount // amount spent on the order
);
---
staking/InfinityStaker.sol:44: event Staked(address indexed user, uint256 amount, Duration duration);
staking/InfinityStaker.sol:45: event DurationChanged(address indexed user, uint256 amount, Duration oldDuration, Duration newDuration);
staking/InfinityStaker.sol:46: event UnStaked(address indexed user, uint256 amount);
staking/InfinityStaker.sol:47: event RageQuit(address indexed user, uint256 totalToUser, uint256 penalty);
token/TimelockConfig.sol:34: event ChangeRequested(bytes32 configId, uint256 value);
token/TimelockConfig.sol:35: event ChangeConfirmed(bytes32 configId, uint256 value);
token/TimelockConfig.sol:36: event ChangeCanceled(bytes32 configId, uint256 value);
token/InfinityToken.sol:35: event EpochAdvanced(uint256 currentEpoch, uint256 supplyMinted);
Tools Used
Manual Review / VSCode
Recommended Mitigation Steps
It is recommended to add indexing to address type parameters.
7. Missing/incomplete natspec comments
Risk
Non-Critical
Impact
Contracts are missing natspec comments which makes code more difficult to read and prone to errors.
Multiple contracts do not check for return values of executed functions. This might lead to accidents and errors since the created transaction is valid but the underlying code did not execute properly.
1. Incorrect check
>=
instead of>
Risk
Low
Impact
Function
InfinityStaker.getRageQuitAmounts
uses require statement to check ifuint256
variabletotalStaked
is bigger or equal to0
.The check should be verifying if
totalStaked
is bigger than0
->
.Proof of Concept
Tools Used
Manual Review / VSCode
Recommended Mitigation Steps
It is recommended to change the
require
check to:2. Missing zero address checks
Risk
Low
Impact
Multiple contracts do not check for zero addresses which might lead to loss of funds, failed transactions and can break the protocol functionality.
Proof of Concept
InfinityStaker.sol
:_tokenAddress
and_infinityTreasury
- https://github.com/code-423n4/2022-06-infinity/blob/765376fa238bbccd8b1e2e12897c91098c7e5ac6/contracts/staking/InfinityStaker.sol#L49destination
- https://github.com/code-423n4/2022-06-infinity/blob/765376fa238bbccd8b1e2e12897c91098c7e5ac6/contracts/staking/InfinityStaker.sol#L345_infinityTreasury
- https://github.com/code-423n4/2022-06-infinity/blob/765376fa238bbccd8b1e2e12897c91098c7e5ac6/contracts/staking/InfinityStaker.sol#L375InfinityExchange.sol
:_WETH
,_matchExecutor
- https://github.com/code-423n4/2022-06-infinity/blob/765376fa238bbccd8b1e2e12897c91098c7e5ac6/contracts/core/InfinityExchange.sol#L104to
- https://github.com/code-423n4/2022-06-infinity/blob/765376fa238bbccd8b1e2e12897c91098c7e5ac6/contracts/core/InfinityExchange.sol#L371destination
- https://github.com/code-423n4/2022-06-infinity/blob/765376fa238bbccd8b1e2e12897c91098c7e5ac6/contracts/core/InfinityExchange.sol#L1221destination
- https://github.com/code-423n4/2022-06-infinity/blob/765376fa238bbccd8b1e2e12897c91098c7e5ac6/contracts/core/InfinityExchange.sol#L1229_currency
- https://github.com/code-423n4/2022-06-infinity/blob/765376fa238bbccd8b1e2e12897c91098c7e5ac6/contracts/core/InfinityExchange.sol#L1235_complication
- https://github.com/code-423n4/2022-06-infinity/blob/765376fa238bbccd8b1e2e12897c91098c7e5ac6/contracts/core/InfinityExchange.sol#L1240_matchExecutor
- https://github.com/code-423n4/2022-06-infinity/blob/765376fa238bbccd8b1e2e12897c91098c7e5ac6/contracts/core/InfinityExchange.sol#L1255InfinityToken.sol
:admin
- https://github.com/code-423n4/2022-06-infinity/blob/765376fa238bbccd8b1e2e12897c91098c7e5ac6/contracts/token/InfinityToken.sol#L38Tools Used
Manual Review / VSCode
Recommended Mitigation Steps
It is recommended to add zero address checks for listed parameters.
3. Missing events
Risk
Low
Impact
Multiple contracts are not implementing events for critical functions. Lack of events makes it difficult for off-chain applications to monitor the protocol.
Proof of Concept
InfinityExchange.sol
:rescueTokens
function event - https://github.com/code-423n4/2022-06-infinity/blob/765376fa238bbccd8b1e2e12897c91098c7e5ac6/contracts/core/InfinityExchange.sol#L1220-L1226rescureETH
function event - https://github.com/code-423n4/2022-06-infinity/blob/765376fa238bbccd8b1e2e12897c91098c7e5ac6/contracts/core/InfinityExchange.sol#L1229-L1232addCurrency
function event - https://github.com/code-423n4/2022-06-infinity/blob/765376fa238bbccd8b1e2e12897c91098c7e5ac6/contracts/core/InfinityExchange.sol#L1235-L1237addComplication
function event - https://github.com/code-423n4/2022-06-infinity/blob/765376fa238bbccd8b1e2e12897c91098c7e5ac6/contracts/core/InfinityExchange.sol#L1240-L1242removeCurrency
function event - https://github.com/code-423n4/2022-06-infinity/blob/765376fa238bbccd8b1e2e12897c91098c7e5ac6/contracts/core/InfinityExchange.sol#L1245-L1247removeComplication
function event - https://github.com/code-423n4/2022-06-infinity/blob/765376fa238bbccd8b1e2e12897c91098c7e5ac6/contracts/core/InfinityExchange.sol#L1250-L1252updateMatchExecutor
function event - https://github.com/code-423n4/2022-06-infinity/blob/765376fa238bbccd8b1e2e12897c91098c7e5ac6/contracts/core/InfinityExchange.sol#L1255-L1257InfinityStaker.sol
:updateStakeLevelThreshold
function event - https://github.com/code-423n4/2022-06-infinity/blob/765376fa238bbccd8b1e2e12897c91098c7e5ac6/contracts/staking/InfinityStaker.sol#L351-L361updatePenalities
function event - https://github.com/code-423n4/2022-06-infinity/blob/765376fa238bbccd8b1e2e12897c91098c7e5ac6/contracts/staking/InfinityStaker.sol#L364-L372updateInfinityTreasury
function event - https://github.com/code-423n4/2022-06-infinity/blob/765376fa238bbccd8b1e2e12897c91098c7e5ac6/contracts/staking/InfinityStaker.sol#L375-L377Tools Used
Manual Review / VSCode
Recommended Mitigation Steps
It is recommended to add missing events to listed functions.
4. Critical address change
Risk
Low
Impact
Changing critical addresses such as ownership should be a two-step process where the first transaction (from the old/current address) registers the new address (i.e. grants ownership) and the second transaction (from the new address) replaces the old address with the new one. This gives an opportunity to recover from incorrect addresses mistakenly used in the first step. If not, contract functionality might become inaccessible.
Proof of Concept
InfinityStaker.sol
Ownable
inheritance - https://github.com/code-423n4/2022-06-infinity/blob/765376fa238bbccd8b1e2e12897c91098c7e5ac6/contracts/staking/InfinityStaker.sol#L15InfinityExchange.sol
Ownable
inheritance - https://github.com/code-423n4/2022-06-infinity/blob/765376fa238bbccd8b1e2e12897c91098c7e5ac6/contracts/core/InfinityExchange.sol#L50InfinityOrderBookComplication.sol
:Ownable
inheritance - https://github.com/code-423n4/2022-06-infinity/blob/765376fa238bbccd8b1e2e12897c91098c7e5ac6/contracts/core/InfinityOrderBookComplication.sol#L14Tools Used
Manual Review / VSCode
Recommended Mitigation Steps
It is recommended to implement two-step process for changing critical addresses.
5. Use scientific notation
Risk
Non-Critical
Impact
Defining big numbers that consists of many digits or performing additional math calculations might lead to accidential errors and mistakes.
Proof of Concept
Tools Used
Manual Review / VSCode
Recommended Mitigation Steps
It is recommended to use scientific notation.
6. Missing indexing for events
Risk
Non-Critical
Impact
Events should index addresses which helps off-chain applications in monitoring the protocol.
Proof of Concept
Tools Used
Manual Review / VSCode
Recommended Mitigation Steps
It is recommended to add indexing to
address
type parameters.7. Missing/incomplete natspec comments
Risk
Non-Critical
Impact
Contracts are missing natspec comments which makes code more difficult to read and prone to errors.
Proof of Concept
InfinityExchange.sol
:@param _WETH
and@param _matchExecutor
- https://github.com/code-423n4/2022-06-infinity/blob/765376fa238bbccd8b1e2e12897c91098c7e5ac6/contracts/core/InfinityExchange.sol#L104@return
- https://github.com/code-423n4/2022-06-infinity/blob/765376fa238bbccd8b1e2e12897c91098c7e5ac6/contracts/core/InfinityExchange.sol#L533@return
- https://github.com/code-423n4/2022-06-infinity/blob/765376fa238bbccd8b1e2e12897c91098c7e5ac6/contracts/core/InfinityExchange.sol#L538@return
- https://github.com/code-423n4/2022-06-infinity/blob/765376fa238bbccd8b1e2e12897c91098c7e5ac6/contracts/core/InfinityExchange.sol#L543@return
- https://github.com/code-423n4/2022-06-infinity/blob/765376fa238bbccd8b1e2e12897c91098c7e5ac6/contracts/core/InfinityExchange.sol#L548@param index
and@return
- https://github.com/code-423n4/2022-06-infinity/blob/765376fa238bbccd8b1e2e12897c91098c7e5ac6/contracts/core/InfinityExchange.sol#L553@param currency
and@return
- https://github.com/code-423n4/2022-06-infinity/blob/765376fa238bbccd8b1e2e12897c91098c7e5ac6/contracts/core/InfinityExchange.sol#L558@param order
and@return
- https://github.com/code-423n4/2022-06-infinity/blob/765376fa238bbccd8b1e2e12897c91098c7e5ac6/contracts/core/InfinityExchange.sol#L1153@param order
and@return
- https://github.com/code-423n4/2022-06-infinity/blob/765376fa238bbccd8b1e2e12897c91098c7e5ac6/contracts/core/InfinityExchange.sol#L1168@param destination
, @param currencyand
@param amount` - https://github.com/code-423n4/2022-06-infinity/blob/765376fa238bbccd8b1e2e12897c91098c7e5ac6/contracts/core/InfinityExchange.sol#L1220@param destination
- https://github.com/code-423n4/2022-06-infinity/blob/765376fa238bbccd8b1e2e12897c91098c7e5ac6/contracts/core/InfinityExchange.sol#L1229@param _currency
- https://github.com/code-423n4/2022-06-infinity/blob/765376fa238bbccd8b1e2e12897c91098c7e5ac6/contracts/core/InfinityExchange.sol#L1235@param _complication
- https://github.com/code-423n4/2022-06-infinity/blob/765376fa238bbccd8b1e2e12897c91098c7e5ac6/contracts/core/InfinityExchange.sol#L1240@param _currency
- https://github.com/code-423n4/2022-06-infinity/blob/765376fa238bbccd8b1e2e12897c91098c7e5ac6/contracts/core/InfinityExchange.sol#L1245@param _complication
- https://github.com/code-423n4/2022-06-infinity/blob/765376fa238bbccd8b1e2e12897c91098c7e5ac6/contracts/core/InfinityExchange.sol#L1250@param _matchExecutor
- https://github.com/code-423n4/2022-06-infinity/blob/765376fa238bbccd8b1e2e12897c91098c7e5ac6/contracts/core/InfinityExchange.sol#L1255@param _wethTransferGasUnits
- https://github.com/code-423n4/2022-06-infinity/blob/765376fa238bbccd8b1e2e12897c91098c7e5ac6/contracts/core/InfinityExchange.sol#L1260@param _protocolFeeBps
- https://github.com/code-423n4/2022-06-infinity/blob/765376fa238bbccd8b1e2e12897c91098c7e5ac6/contracts/core/InfinityExchange.sol#L1266InfinityOrderBookComplication.sol
:@param sell
,@param buy
,@return
- https://github.com/code-423n4/2022-06-infinity/blob/765376fa238bbccd8b1e2e12897c91098c7e5ac6/contracts/core/InfinityOrderBookComplication.sol#L168-L169@param sell
,@param buy
and@return
- https://github.com/code-423n4/2022-06-infinity/blob/765376fa238bbccd8b1e2e12897c91098c7e5ac6/contracts/core/InfinityOrderBookComplication.sol#L182@param sell
, @param buy,
@param constructedNftsand
@return` - https://github.com/code-423n4/2022-06-infinity/blob/765376fa238bbccd8b1e2e12897c91098c7e5ac6/contracts/core/InfinityOrderBookComplication.sol#L191-L192@param makeOrder
,@param takerItems
and@return
- https://github.com/code-423n4/2022-06-infinity/blob/765376fa238bbccd8b1e2e12897c91098c7e5ac6/contracts/core/InfinityOrderBookComplication.sol#L208-L209@param orders
and@return
- https://github.com/code-423n4/2022-06-infinity/blob/765376fa238bbccd8b1e2e12897c91098c7e5ac6/contracts/core/InfinityOrderBookComplication.sol#L316-L317@param order
and@return
- https://github.com/code-423n4/2022-06-infinity/blob/765376fa238bbccd8b1e2e12897c91098c7e5ac6/contracts/core/InfinityOrderBookComplication.sol#L329-L330InfinityStaker.sol
:@param _tokenAddress
and@param _infinityTreasury
- https://github.com/code-423n4/2022-06-infinity/blob/765376fa238bbccd8b1e2e12897c91098c7e5ac6/contracts/staking/InfinityStaker.sol#L49@param user
,@param amount
,@param noVesting
,@param vestedThreeMonths
,@param vestedSixMonths
,@param vestedTwelveMonths
- https://github.com/code-423n4/2022-06-infinity/blob/765376fa238bbccd8b1e2e12897c91098c7e5ac6/contracts/staking/InfinityStaker.sol#L287-L290@param user
- https://github.com/code-423n4/2022-06-infinity/blob/765376fa238bbccd8b1e2e12897c91098c7e5ac6/contracts/staking/InfinityStaker.sol#L327-L328@param destination
- https://github.com/code-423n4/2022-06-infinity/blob/765376fa238bbccd8b1e2e12897c91098c7e5ac6/contracts/staking/InfinityStaker.sol#L344-L345@param stakeLevel
,@param threshold
- https://github.com/code-423n4/2022-06-infinity/blob/765376fa238bbccd8b1e2e12897c91098c7e5ac6/contracts/staking/InfinityStaker.sol#L350-L351@param threeMonthPenalty
,@param sixMonthPenalty
,@param twelveMonthPenalty
- https://github.com/code-423n4/2022-06-infinity/blob/765376fa238bbccd8b1e2e12897c91098c7e5ac6/contracts/staking/InfinityStaker.sol#L363-L364@param _infinityTreasury
- https://github.com/code-423n4/2022-06-infinity/blob/765376fa238bbccd8b1e2e12897c91098c7e5ac6/contracts/staking/InfinityStaker.sol#L374-L375InfinityToken.sol
:TimelockConfig.sol
:Tools Used
Manual Review / VSCode
Recommended Mitigation Steps
It is recommended to add missing natspec comments.
8. Typos
Risk
Non-Critical
Impact
Code and comments contain typos which makes code more difficult to read and prone to errors.
Proof of Concept
InfinityStaker.sol
:Untake tokens
- https://github.com/code-423n4/2022-06-infinity/blob/765376fa238bbccd8b1e2e12897c91098c7e5ac6/contracts/staking/InfinityStaker.sol#L112vestedsixMonth
, expected camel casevestedSixMonths
- https://github.com/code-423n4/2022-06-infinity/blob/765376fa238bbccd8b1e2e12897c91098c7e5ac6/contracts/staking/InfinityStaker.sol#L120Tools Used
Manual Review / VSCode
Recommended Mitigation Steps
It is recommended to correct listed typos.
8. Check return values
Risk
Non-Critical
Impact
Multiple contracts do not check for return values of executed functions. This might lead to accidents and errors since the created transaction is valid but the underlying code did not execute properly.
Proof of Concept
InfinityExchange.sol
:_currencies.add
- https://github.com/code-423n4/2022-06-infinity/blob/765376fa238bbccd8b1e2e12897c91098c7e5ac6/contracts/core/InfinityExchange.sol#L1236_complications.add
- https://github.com/code-423n4/2022-06-infinity/blob/765376fa238bbccd8b1e2e12897c91098c7e5ac6/contracts/core/InfinityExchange.sol#L1241_currencies.remove
- https://github.com/code-423n4/2022-06-infinity/blob/765376fa238bbccd8b1e2e12897c91098c7e5ac6/contracts/core/InfinityExchange.sol#L1246_complications.remove
- https://github.com/code-423n4/2022-06-infinity/blob/765376fa238bbccd8b1e2e12897c91098c7e5ac6/contracts/core/InfinityExchange.sol#L1251InfinityToken.sol
:_configSet.add
- https://github.com/code-423n4/2022-06-infinity/blob/765376fa238bbccd8b1e2e12897c91098c7e5ac6/contracts/token/TimelockConfig.sol#L55_configSet.add
- https://github.com/code-423n4/2022-06-infinity/blob/765376fa238bbccd8b1e2e12897c91098c7e5ac6/contracts/token/TimelockConfig.sol#L67Tools Used
Manual Review / VSCode
Recommended Mitigation Steps
It is recommended to check the return values of executed functions.