KlimaStakingDistributor_v4 Manual Review Findings

KSD-01M: Improper Accumulation of Rewards

Description:

The nextRewardFor function does not properly accumulate rewards if multiple ones are specified for a particular recipient.

Example:

Copy
446function nextRewardFor( address _recipient ) public view returns ( uint ) {
447    uint reward;
448    for ( uint i = 0; i < info.length; i++ ) {
449        if ( info[ i ].recipient == _recipient ) {
450            reward = nextRewardAt( info[ i ].rate );
451        }
452    }
453    return reward;
454}

Recommendation:

We advise the function to properly sum the results of nextRewardAt invocations to ensure it operates as intended.

Alleviation:

The KlimaDAO team responded by stating that there is only one address that receives a reward. In such a case, we advise the reward to be returned directly instead to optimize the codebase even further.

KSD-02M: Improper Policy Renouncation

Description:

The renouncePolicy function does not properly clear out any pending _newPolicy, thus permitting a malicious policy holder to fake renouncing their rights and then reclaim them at a later date.

Example:

Copy
312function renouncePolicy() public virtual override onlyPolicy() {
313    emit OwnershipTransferred( _policy, address(0) );
314    _policy = address(0);
315}
316
317function pushPolicy( address newPolicy_ ) public virtual override onlyPolicy() {
318    require( newPolicy_ != address(0), "Ownable: new owner is the zero address");
319    _newPolicy = newPolicy_;
320}
321
322function pullPolicy() public virtual override {
323    require( msg.sender == _newPolicy );
324    emit OwnershipTransferred( _policy, _newPolicy );
325    _policy = _newPolicy;
326}

Recommendation:

We advise the function to properly clear out the _newPolicy data entry.

Alleviation:

The _newPolicy entry is now properly cleared out when renouncing thereby alleviating this exhibit.

KSD-03M: Ungraceful Handling of High Adjustment Rates

Description:

The adjustment.rate is meant to represent a step-by-step reduction or increase of the reward rate for a particular recipient, however, there can be a case where the info[_index].rate is smaller than the step which would render the adjust operation impossible and thus cause the full distribute hook to fail.

Example:

Copy
419} else { // if rate should decrease
420    info[ _index ].rate = info[ _index ].rate.sub( adjustment.rate ); // lower rate
421    if ( info[ _index ].rate <= adjustment.target ) { // if target met
422        adjustments[ _index ].rate = 0; // turn off adjustment
423    }
424}

Recommendation:

We advise the reduction of a particular rate to be gracefully handled whereby if the reduction is greater than the current rate the rate should be set to zero.

Alleviation:

The KlimaDAO team responded by stating that this is by design and a controlled contract parameter and as such no graceful handling is necessary.

KSD-04M: Inexistent Validation of Entry Validity

Description:

The removeRecipient and setAdjustment functions do not actually validate whether there is an existing entry in the _index they are operating in, leading to adjustments for inexistent entries / future ones or removal of inexistent entries.

Example:

Copy
473/**
474    @notice removes recipient for distributions
475    @param _index uint
476    @param _recipient address
477 */
478function removeRecipient( uint _index, address _recipient ) external onlyPolicy() {
479    require( _recipient == info[ _index ].recipient );
480    info[ _index ].recipient = address(0);
481    info[ _index ].rate = 0;
482}
483
484/**
485    @notice set adjustment info for a collector's reward rate
486    @param _index uint
487    @param _add bool
488    @param _rate uint
489    @param _target uint
490 */
491function setAdjustment( uint _index, bool _add, uint _rate, uint _target ) external onlyPolicy() {
492    adjustments[ _index ] = Adjust({
493    add: _add,
494    rate: _rate,
495    target: _target
496    });
497}

Recommendation:

We advise both functions to properly validate that an info entry exists by evaluating its recipient.

Alleviation:

Both functions now properly validate that a recipient exists in the particular _index.

KSD-05M: Inexistent Validation of Reward Rate

Description:

The addRecipient function does not validate that the _rewardRate set does not exceed the maximum achievable which is 1000000.

Example:

Copy
460/**
461    @notice adds recipient for distributions
462    @param _recipient address
463    @param _rewardRate uint
464 */
465function addRecipient( address _recipient, uint _rewardRate ) external onlyPolicy() {
466    require( _recipient != address(0) );
467    info.push( Info({
468    recipient: _recipient,
469    rate: _rewardRate
470    }));
471}

Recommendation:

We advise such validation to be imposed to prevent arbitrarily high reward rates.

Alleviation:

The KlimaDAO team considered this exhibit but opted to retain the codebase in its current state.