Core Contracts

Summary

This issue involves the LendingPool contract incorrectly using msg.sender instead of its own address (address(this)) as the owner parameter when calling the vault’s withdrawal function. Since the vault recognizes address(this) as the entity that holds the shares (due to the deposit transaction), passing msg.sender violates the expected ownership logic. As a result, the vault either reverts—if it strictly checks ownership—or silently allows withdrawals under incorrect assumptions, potentially compromising the security model of the protocol.

Vulnerability Details

Within the LendingPool contract, the _withdrawFromVault function incorrectly passes msg.sender rather than address(this) as the owner parameter when invoking the vault’s withdraw function. During the deposit phase, address(this) is designated as the share owner. Consequently, calling withdraw with msg.sender causes the vault to reject the transaction if it strictly verifies share ownership. This inconsistency interrupts the expected rebalancing and withdrawal process. If the vault does not validate ownership thoroughly, there is a risk that unauthorized addresses could withdraw shares, compromising the protocol's security model.

Impact

If the vault enforces correct share ownership, any attempt to withdraw using msg.sender instead of address(this) will fail, preventing the LendingPool from accessing necessary liquidity and breaking the rebalancing mechanism. In cases where the vault does not strictly check the owner parameter, an unauthorized address could potentially withdraw shares, compromising the protocol’s security and allowing unintended funds transfer.

Proof of Concept

The _withdrawFromVault function currently calls:

However, the typical curveVault.withdraw(...) signature expects two critical parameters:

1. recipient: the address that will receive the withdrawn tokens.

2. owner: the address holding the shares in the vault, thus authorized to initiate the withdrawal.

When depositing, the contract registers address(this) (the LendingPool contract itself) as the owner of the shares, because the code calls curveVault.deposit(amount, address(this)).
Therefore, when withdrawing, the owner should also be address(this).

In the current code, msg.sender is incorrectly used as the owner. This inconsistency can cause the vault to reject the transaction or, if it unexpectedly accepts it, compromise the security assumption that only address(this) (the legitimate depositor) can withdraw those funds.

Code Analysis

• LendingPool.sol - _depositIntoVault

• LendingPool.sol - _withdrawFromVault

Below is a simplified excerpt highlighting the deposit and withdrawal logic to the vault (_depositIntoVault and _withdrawFromVault) with the finding marked by (!) for clarity:

Explanation

• In _depositIntoVault, the LendingPool deposits funds, thereby becoming the owner of those shares in the vault (curveVault).

• In _withdrawFromVault, the withdrawal must be initiated by the same entity that owns those shares. If msg.sender is used, the vault will not recognize the caller as the legitimate owner.

This flaw can cause the withdraw(...) call to fail if the vault strictly checks share ownership. Even if it does not fail due to some vault-specific quirk, it breaks the security assumption that only address(this) can manage the deposited funds.

Vulnerable Scenario

1. A user triggers a function (e.g., _rebalanceLiquidity()) that internally attempts to withdraw funds by calling _withdrawFromVault.

2. The internal call uses curveVault.withdraw(amount, address(this), msg.sender, 0, new address[](0));.

3. The vault detects that msg.sender is not the actual owner of the shares, causing the transaction to fail.

   • Result: Liquidity becomes unavailable, and the rebalancing mechanism breaks.

4. In a rarer situation, if the vault does not validate ownership properly, it could introduce inconsistencies in share accounting.

Test and Result

The test confirms that when the LendingPool tries to withdraw from the MockVault, it incorrectly uses msg.sender instead of address(this) as the owner parameter. Consequently, the vault reverts with "WrongOwner()". This validates the logic flaw: only the entity that deposited (i.e., address(this)) can withdraw those shares, ensuring consistent ownership.

MockVault is a simplified contract that mimics a typical vault’s deposit and withdrawal mechanisms. It assigns "shares" upon deposit and expects the owner parameter in the withdraw function to match address(this). If a different address attempts to withdraw, it fails with "WrongOwner()". This setup allows to isolate and confirm the incorrect owner parameter usage in the LendingPool.

TestLendingPool inherits from the actual LendingPool and exposes the normally internal functions (_depositIntoVault and _withdrawFromVault) as public (testDepositIntoVault and testWithdrawFromVault). This lets the test directly call those methods and verify that using msg.sender instead of address(this) triggers the expected error in the MockVault.

• Add the following Mock to contracts/mocks/MockVault.sol

• Add the following Mock to contracts/mocks/TestLendingPool.sol

• Add the following test to test/unit/core/pools/LendingPool/LendingPool.test.js

Confirmation

This test conclusively demonstrates that the withdrawal mechanism fails if the vault expects address(this) as the owner, yet the LendingPool passes msg.sender instead. The vault reverts with "WrongOwner()", confirming the misalignment in ownership parameters and validating the logical error in _withdrawFromVault.

Tools Used

Manual Code Review was the primary approach, focusing on the vault interaction code paths and parameter usage during internal deposit and withdrawal processes.

Recommendations

Ensuring the LendingPool contract always uses its own address (address(this)) instead of msg.sender when withdrawing from the vault will resolve the ownership discrepancy.

• Remove the call that passes msg.sender as the owner.

• Add address(this) as the correct owner parameter to maintain consistent ownership logic with the deposit flow.