Obsidian - Malicious pool deployer can set a malicious interest rate contract to lock funds of vault depositors

[sherlock-admin2]

Obsidian

High

Malicious pool deployer can set a malicious interest rate contract to lock funds of vault depositors

Summary

Once vault depositors have deposited funds into a pool, a malicious pool creator can upgrade the interestRateStrategy contract (using PoolConfigurator.setReserveInterestRateStrategyAddress() to make all calls to it revert.

As a result any function of the protocol that calls updateInterestRates() will revert because updateInterestRates() makes the following call:

(vars.nextLiquidityRate, vars.nextBorrowRate) = IReserveInterestRateStrategy(_reserve.interestRateStrategyAddress)
      .calculateInterestRates(
      /* PARAMS */
    );

The main impact is that now withdrawals will revert because executeWithdraw() calls updateInterestRates() which will always revert, so the funds that vault users deposited into this pool are lost forever.

Root Cause

Allowing the pool deployer to specify any interestRateStrategyAddress

Internal pre-conditions

No response

External pre-conditions

No response

Attack Path

1. Vault users deposit into the pool
2. the deployer sets their interestRateStrategy contract to make all calls to it revert
3. All calls to withdraw funds from the pool will revert, the vault depositors have lost their funds

Impact

All the funds deposited to the pool from the vault will be lost

PoC

No response

Mitigation

Use protocol whitelisted interest rate calculation contracts

[nevillehuang]

Invalid, require malicious admin

Essentially we expect all permissioned actors to behave rationally.

[iamnmt]

Escalate

Per the contest's README

There are two set of actors. Actors who manage pools and actors who mange vaults. If an action done by one party causes the other party to suffer losses we'd want to consider that.

This statement makes this issue valid.

[sherlock-admin3]

Escalate

Per the contest's README

There are two set of actors. Actors who manage pools and actors who mange vaults. If an action done by one party causes the other party to suffer losses we'd want to consider that.

This statement makes this issue valid.

You've created a valid escalation!

To remove the escalation from consideration: Delete your comment.

You may delete or edit your escalation comment anytime before the 48-hour escalation window closes. After that, the escalation becomes final.

[zarkk01]

IMO, the issue is invalid due to this statement of the sponsor.

Essentially we expect all permissioned actors to behave rationally.

It is, absolutely, irrational for a deployer to set a malicious interest rate contract since he has nothing to earn out of this behaviour.

[0xSpearmint]

The permissioned actors the protocol refers to are the protocol controlled PoolConfigurator and owner roles. They can be expected to act rationally.

This issue involves a malicious pool deployer (which can be anyone).

Deploying pools is permission-less, which is why the protocol was interested in such issues as they clearly stated in the README:

There are two set of actors. Actors who manage pools and actors who mange vaults. If an action done by one party causes the other party to suffer losses we'd want to consider that.

[cvetanovv]

I agree with the escalation of this issue to be High severity. For more information on what I think about the rule, you can see this comment: #234 (comment)

Planning to accept the escalation and make this issue High severity.

[WangSecurity]

Result:
High
Has duplicates

[sherlock-admin2]

Escalations have been resolved successfully!

Escalation status:

• iamnmt: accepted

[DemoreXTess]

@cvetanovv Can we reconsider this issue per this comment : #234 (comment)_

The report wrongly states that the funds are locked forever. ZeroLend has permission to make changes on the pools. Users can get back their funds after adjustment by ZeroLend

[0xSpearmint]

The referenced comment is not accurate.

Even if the protocol sets a new IRM through this only configurator function, the pool admin can instantly change it back to the malicious IRM using this only pool admin function.

[cvetanovv]

I agree with @0xSpearmint comment. Even if ZeroLend makes any changes, the malicious pool admin can immediately roll back the previous configuration.

[DemoreXTess]

@cvetanovv
Is there a problem in this one, it says escalation resolved with has duplicates but the label is not added.

[cvetanovv]

@DemoreXTess That's not a problem. The labels of duplicate issues will be added after all escalations are resolved.