SmartRewardsDistributor Static Analysis Findings

SRD-01S: Inexistent Event Emissions

Type	Severity	Location
Language Specific		SmartRewardsDistributor.sol:L105-L112, L259-L264, L269-L271, L335-L338, L350-L352

Description:

The linked functions adjust sensitive contract variables yet do not emit an event for it.

Example:

Copy
269function setAbandonmentBlockTimeframe(uint256 newBlockTimeframe) external onlyOwner {
270    abandonmentBlockTimeframe = newBlockTimeframe;
271}

Recommendation:

We advise an event to be declared and correspondingly emitted for each function to ensure off-chain processes can properly react to this system adjustment.

Alleviation (e31556397b9d321737ff47809b572383dda4a2aa):

The MerkleRootHashUpdated, FeeDiscountChanged, AbandonmnetBlockTimeframeChanged, SteerFeesChanged, and FeeCollectionAddressChanged events were introduced to the codebase and are correspondingly emitted in the SmartRewardsDistributor::orchestratorUpdateRootHash, SmartRewardsDistributor::setFeeDiscount, SmartRewardsDistributor::setAbandonmentBlockTimeframe, SmartRewardsDistributor::setSteerFee, and SmartRewardsDistributor::setFeeCollectionAddress functions respectively, addressing this exhibit in full.

SRD-02S: Inexistent Sanitization of Input Addresses

Type	Severity	Location
Input Sanitization		SmartRewardsDistributor.sol:L82-L98, L350-L352

Description:

The linked function(s) accept address arguments yet do not properly sanitize them.

Impact:

The presence of zero-value addresses, especially in constructor implementations, can cause the contract to be permanently inoperable. These checks are advised as zero-value inputs are a common side-effect of off-chain software related bugs.

Example:

Copy
82function initialize(
83    address _orchestratorRelay,
84    uint256 _steerFee,
85    address _feeCollectionAddress,
86    uint256 _abandonmentBlockTimeframe
87) public initializer {
88    __Ownable_init();
89    __UUPSUpgradeable_init();
90    // @note will need to be known on initialization, deploy dynamic jobs contract first, then register
91    orchestratorRelay = _orchestratorRelay;
92    steerFee = _steerFee > MAX_STEER_FEE ? MAX_STEER_FEE : _steerFee;
93    feeCollectionAddress = _feeCollectionAddress;
94    abandonmentBlockTimeframe = _abandonmentBlockTimeframe;
95
96    // Set last updated to current block to prevent canceled campaigns not registering
97    lastBlockUpdate = block.number;
98}

Recommendation:

We advise some basic sanitization to be put in place by ensuring that each address specified is non-zero.

Alleviation (e31556397b9d321737ff47809b572383dda4a2aa):

All input argument(s) of the SmartRewardsDistributor::initialize, and SmartRewardsDistributor::setFeeCollectionAddress functions are adequately sanitized as non-zero in the latest in-scope revision of the codebase, addressing this exhibit.

SRD-03S: Improper Invocations of EIP-20 transfer / transferFrom

Type	Severity	Location
Standard Conformity		SmartRewardsDistributor.sol:L170, L211-L215, L221-L225, L290-L293, L301-L304

Description:

The linked statements do not properly validate the returned bool values of the EIP-20 standard transfer & transferFrom functions. As the standard dictates, callers must not assume that false is never returned.

Impact:

If the code mandates that the returned bool is true, this will cause incompatibility with tokens such as USDT / Tether as no such bool is returned to be evaluated causing the check to fail at all times. On the other hand, if the token utilized can return a false value under certain conditions but the code does not validate it, the contract itself can be compromised as having received / sent funds that it never did.

Example:

Copy
170IERC20(campaign.rewardToken).transfer(user, outbound);

Recommendation:

Since not all standardized tokens are EIP-20 compliant (such as Tether / USDT), we advise a safe wrapper library to be utilized instead such as SafeERC20 by OpenZeppelin to opportunistically validate the returned bool only if it exists in each instance.

Alleviation (e31556397b9d321737ff47809b572383dda4a2aa):

The SafeERC20 library of the OpenZeppelin dependency is now properly imported to the codebase and its SafeERC20::safeTransferFrom & SafeERC20::safeTransfer functions are correctly invoked in place of all potentially unhandled EIP-20 ERC20::transfer & ERC20::transferFrom invocations, addressing this exhibit.