Missing check for Pool state on several functions in Pool.sol

[code423n4]

Handle

0xRajeev

Vulnerability details

Impact

The Pool may be in three states: Initialized, Finalized and Deactivated as indicated by the enum State variable. While a couple of functions such as fundLoan() and deposit() check against a valid Pool state i.e. Finalized using _isValidState(State.Finalized), most other functions miss this check. This could cause unexpected protocol behavior if such functions are triggered in invalid Pool states e.g. Deactivated.

Examples of such functions missing this Pool state validity check are triggerDefault(), claim(), withdraw() and withdrawFunds().

Proof of Concept

https://github.com/maple-labs/maple-core/blob/355141befa89c7623150a83b7d56a5f5820819e9/contracts/Pool.sol#L50

https://github.com/maple-labs/maple-core/blob/355141befa89c7623150a83b7d56a5f5820819e9/contracts/Pool.sol#L168 https://github.com/maple-labs/maple-core/blob/355141befa89c7623150a83b7d56a5f5820819e9/contracts/Pool.sol#L185 https://github.com/maple-labs/maple-core/blob/355141befa89c7623150a83b7d56a5f5820819e9/contracts/Pool.sol#L392 https://github.com/maple-labs/maple-core/blob/355141befa89c7623150a83b7d56a5f5820819e9/contracts/Pool.sol#L424

Tools Used

Manual Analysis

Recommended Mitigation Steps

Add _isValidState(State.Finalized) check to all such functions specified above.

[lucas-manuel]

Informational, will address, not really a bug.

[Arachnid]

Submitter has not demonstrated how this can be exploited, but these seem like important checks to be omitting and may well result in invariants being violated. In the absence of a specific exploit vector, I'm awarding this as Low.

[satyamakgec]

So we did an analysis again and here are some data points -

• triggerDefault() -> It will only be called by the Pool Delegate and it internally checks whether the Pool (precicesly DebtLocker) has the 20 % of supplied LoanFDTs If yes then procced otherwise fail. So in the Initialized state it will not be callable as there is no fund provided to loan so not LoanFDT in the debtlocker, While if the loan get defaulted once then loan will automatically get closed so triggerDefault() will revert in that case.

• withdraw() -> It has again implicit check of the pool state as there is no way to get the poolFDT if the Pool is in the Initialized state while if Pool is in the Finalized state then again there is no harm as user can withdraw its funds if they have otherwise it will not be possible.

• withdrawFunds() -> This is simillar as above if you have entitled amount then only it will works otherwise it will be a just a gas waste of the msg.sender.

• claim() -> It is a permissioned function (only be callable by Pool delegate) even if it get called during Initialized or after Finalized state then nothing happen only be the waste of gas which I think it is okay in this case as PD is very well aware about the process.

We could add the statechecks eventually but it doesn't give us any extra benefit although it does increase the size of the PoolFactory contract bytecode that we don't want as we are already on the verge of 24 KB