KlimaTreasury Manual Review Findings

KTY-01M: Insecure Management of Reserve & Liquidity Tokens

Description:

The Tokens-suffixed arrays of reserve and liquidity tokens are improperly maintained which can cause huge discrepancies to the totalReserves measured in the auditReserves function. As an example, an asset being a reserve and liquidity token at the same time will cause an incorrect duplicate measurement of its balance value.

Example:

Copy
774} else if ( _managing == MANAGING.LIQUIDITYTOKEN ) { // 5
775    if ( requirements( LiquidityTokenQueue, isLiquidityToken, _address ) ) {
776        LiquidityTokenQueue[ _address ] = 0;
777        if( !listContains( liquidityTokens, _address ) ) {
778            liquidityTokens.push( _address );
779        }
780    }
781    result = !isLiquidityToken[ _address ];
782    isLiquidityToken[ _address ] = result;
783    bondCalculator[ _address ] = _calculator;
784
785} else if ( _managing == MANAGING.LIQUIDITYMANAGER ) { // 6

Recommendation:

We advise all actions modifying those arrays to properly check for duplicates by preventing re-setting the same permission for an address to true.

Alleviation:

The KlimaDAO team stated that the address lists are carefully maintained and are behind a 24 hour timelock that prevents improper configuration from being set from the contract.

KTY-02M: Weak Debt Position Validation

Description:

The incurDebt function mandates that the caller has a sufficient balance of sKLIMA to create the debt position, however, sKLIMA is a freely transferrable asset that should only be used as a data point when it cannot be transferred.

Example:

Copy
548uint maximumDebt = IERC20( sKLIMA ).balanceOf( msg.sender ); // Can only borrow against sKLIMA held
549uint availableDebt = maximumDebt.sub( debtorBalance[ msg.sender ] );
550require( value <= availableDebt, "Exceeds debt limit" );

Recommendation:

We advise the sKLIMA to either be held in custody or for some other similar mechanism to be put in place as the current debt mechanism is circumventable.

Alleviation:

The KlimaDAO team stated that this is a design choice and that the validation does not need to be escrowed.

KTY-03M: Improper Token Status Assumption

Description:

The code incorrectly assumes that if the input _token is not a reserve token it is a liquidity token and vice versa, however, this is not guaranteed by any aspect of the codebase.

Example:

Copy
500if ( isReserveToken[ _token ] ) {
501    require( isReserveDepositor[ msg.sender ], "Not approved as a Reserve Depositor" );
502} else {
503    require( isLiquidityDepositor[ msg.sender ], "Not approved as a Liquidity Depositor" );
504}

Recommendation:

We advise the require check within the else branch to also validate the isLiquidityToken or isReserveToken data entry.

Alleviation:

The KlimaDAO team stated that all actions sit behind a 24 hour timelock and as such the assumption will hold true given that the contract will be invoked properly under all circumstances.

KTY-04M: Potentially Unsafe Primitive Evaluation

Description:

The evaluation of a particular token's value when the token is not a LIQUIDITYTOKEN is performed by a simple decimal-based conversion.

Example:

Copy
669function valueOf( address _token, uint _amount ) public view returns ( uint value_ ) {
670    if ( isReserveToken[ _token ] ) {
671        // convert amount to match KLIMA decimals
672        value_ = _amount.mul( 10 ** IERC20( KLIMA ).decimals() ).div( 10 ** IERC20( _token ).decimals() );
673    } else if ( isLiquidityToken[ _token ] ) {
674        value_ = IBondCalculator( bondCalculator[ _token ] ).valuation( _token, _amount );
675    }
676}

Recommendation:

Apart from considering these tokens equal in value, it also directly relies on the presence of the decimals operator on the token which at times may not be available as it is an OPTIONAL member of the EIP-20 standard. We advise this particular trait to be considered carefully in the overall system as checks can be imposed at the inclusion level to prevent non-compliant tokens from being added.

Alleviation:

The KlimaDAO team stated that this particular calculation is by design and as such should not be considered non-standard.