QA Report #98
Labels
bug
Something isn't working
QA (Quality Assurance)
Assets are not at risk. State handling, function incorrect as to spec, issues with clarity, syntax
QA (LOW & NON-CRITICAL)
[L-01] Missing checks for contract existence
OpenZeppelin's
safeTransfer
andsafeTransferFrom
functions usefunctionCall
of Address.sol which is a low level call.However, it's stated that the target must be a contract and code existence should be checked prior using. Reference It's advised to check code existence prior calling safeTransfer or safeTransferFrom as below;
In InfinityExchange.sol;
In InfinityStaker.sol;
[L-02] Missing of zero address checks during construction
There are no
address(0)
or Zero value check at the constructors of:InfinityExchange.sol, InfinityStaker.sol, InfinityToken.sol
[L-03] Missing indexes in important events
Non of the events in InfinityExchange.sol, InfinityToken.sol has indexed parameter.
[L-04] Redundant statement
The team might consider to remove the line below since the condition is already checked in
Signaturechecker.verify
function;https://github.com/code-423n4/2022-06-infinity/blob/765376fa238bbccd8b1e2e12897c91098c7e5ac6/contracts/core/InfinityExchange.sol#L523
[L-05] Critical address / parameter changes
Critical address / value changes are not done in two steps for the following methods:
At InfinityStaker.sol;
updateInfinityTreasury()
,updateStakeLevelThreshold()
,updatePenalties()
,Changing critical addresses in contracts 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 (i.e. claims ownership). This gives an opportunity to recover from incorrect addresses mistakenly used in the first step. If not, contract functionality might become inaccessible. Reference, Reference
[L-06] Signature malleability of EVM's ecrecover in verify()
The contract uses ecrecover() function after many function triggering in InfinityExchange.sol's
verifyOrderSig()
. EVM's ecrecover is susceptible to signature malleability which allows replay attacks, but using OpenZeppelin’s ECDSA library can be mitigation for this. Reference[L-07] Zero check for denominator at contract level
At InfinityExchange.sol matchOneToOneOrders() function, the parameter
makerOrders1.length
should be checked against being zero. No matter it's checked at the interface level, if somehow it's missed and the calldata was input as 0, it will revert.[N-01] All capitals are used for CONSTANTS
All capital letters in a variable is used for constanst/immutable variables. However, this was not followed for the below;
https://github.com/code-423n4/2022-06-infinity/blob/765376fa238bbccd8b1e2e12897c91098c7e5ac6/contracts/core/InfinityExchange.sol#L1161
The text was updated successfully, but these errors were encountered: