[CRITICAL] LpCollateralRouter Accounting Mismatch Leading to Vault Insolvency

[Aliali97]

Summary

Severity: Critical
Bounty: Up to $2,500,000
Affected Contract: LpCollateralRouter.sol and all contracts inheriting it: HypERC20Collateral, HypNative

The LpCollateralRouter contract implements an ERC4626 vault that also serves as a cross-chain collateral router. It maintains an internal accounting variable lpAssets to track total assets held by the vault. However, when tokens leave the contract via the cross-chain _handle() function (inherited from TokenRouter), lpAssets is never decremented. This creates an accounting mismatch where totalAssets() reports a higher value than the actual token balance, leading to vault insolvency.

Vulnerability Details

Root Cause

The LpCollateralRouter contract overrides ERC4626's _deposit() and _withdraw() to track collateral via the lpAssets variable:

// LpCollateralRouter.sol:53-69
function _deposit(address caller, address receiver, uint256 assets, uint256 shares) internal override {
    _transferFromSender(assets);
    lpAssets += assets;          // ✅ Incremented on deposit
    _mint(receiver, shares);
    emit Deposit(caller, receiver, assets, shares);
}

// LpCollateralRouter.sol:72-92
function _withdraw(address caller, address receiver, address owner, uint256 assets, uint256 shares) internal override {
    _burn(owner, shares);
    lpAssets -= assets;          // ✅ Decremented on withdraw
    _transferTo(receiver, assets);
    emit Withdraw(caller, receiver, owner, assets, shares);
}

[README.md](https://github.com/user-attachments/files/30156813/README.md)
[run-poc.sh](https://github.com/user-attachments/files/30156814/run-poc.sh)

[README.md](https://github.com/user-attachments/files/30156818/README.md)

However, the inherited _handle() function from TokenRouter transfers tokens OUT of the contract when receiving cross-chain messages, but does NOT update lpAssets:

// TokenRouter.sol:623-637 (inherited by LpCollateralRouter, NOT overridden)
function _handle(uint32 _origin, bytes32, bytes calldata _message) internal virtual override {
    bytes32 recipient = _message.recipient();
    uint256 amount = _message.amount();
    emit ReceivedTransferRemote(_origin, recipient, amount);
    _transferTo(recipient.bytes32ToAddress(), _inboundAmount(amount));
    // ❌ lpAssets is NOT decremented here!
}

Additionally, transferRemote() (outbound bridging) also moves tokens out of the contract via _transferFromSender() in _calculateFeesAndCharge(), but this path also does NOT update lpAssets.

Affected Contracts

Contract	Inherits From	Impact
HypERC20Collateral	LpCollateralRouter	Deployed on multiple chains
HypNative	LpCollateralRouter	Deployed on multiple chains

Neither HypERC20Collateral nor HypNative override _handle() to fix the accounting.

Proof of Concept

Scenario: Vault Insolvency via Cross-Chain Bridge

Setup:
- HypERC20Collateral contract on Chain B holds USDC as collateral
- Alice is an LP who deposits into the vault
- Bob bridges tokens from Chain A to Chain B

Step 1: Alice deposits 1,000 USDC via deposit()
  - lpAssets = 1,000
  - contract.balance = 1,000
  - Alice receives shares proportional to 1,000 USDC

Step 2: Cross-chain message arrives (incoming bridge from Chain A)
  - _handle() is called
  - _transferTo(recipient, 500) → sends 500 USDC to recipient
  - contract.balance = 500 (tokens left the contract)
  - lpAssets = 1,000 (NOT UPDATED!)

Step 3: Alice tries to withdraw
  - totalAssets() returns lpAssets = 1,000
  - Alice's shares represent 1,000 USDC worth
  - But contract only has 500 USDC
  - Result: REVERT (insufficient balance) or partial withdrawal
  - Alice's funds are effectively locked/stolen

Step 4: Any subsequent LP trying to withdraw
  - Same issue - contract is insolvent
  - totalAssets() overstates actual holdings
  - Later withdrawers lose their funds entirely

Impact Calculation

If X tokens are bridged out via _handle() without updating lpAssets:

• totalAssets() is overstated by X
• All LP share valuations are inflated
• First withdrawer may succeed (if amount ≤ actual balance), but subsequent withdrawers will fail
• Last withdrawers lose their funds entirely

Suggested Fix

Override _handle() in LpCollateralRouter to decrement lpAssets when tokens leave:

function _handle(
    uint32 _origin,
    bytes32 _sender,
    bytes calldata _message
) internal virtual override {
    uint256 amount = _inboundAmount(_message.amount());
    lpAssets -= amount;  // FIX: Decrement lpAssets when tokens leave
    super._handle(_origin, _sender, _message);
}

Additionally, if transferRemote() (outbound) also moves collateral out of the contract, lpAssets should be decremented there as well, or _transferTo should be overridden to track all outgoing transfers.

References

• Vulnerable File: https://github.com/hyperlane-xyz/hyperlane-monorepo/blob/main/solidity/contracts/token/libs/LpCollateralRouter.sol
• Base Contract: https://github.com/hyperlane-xyz/hyperlane-monorepo/blob/main/solidity/contracts/token/libs/TokenRouter.sol
• Affected: HypERC20Collateral: https://github.com/hyperlane-xyz/hyperlane-monorepo/blob/main/solidity/contracts/token/HypERC20Collateral.sol
• Affected: HypNative: https://github.com/hyperlane-xyz/hyperlane-monorepo/blob/main/solidity/contracts/token/HypNative.sol
• Immunefi Bounty: https://immunefi.com/bug-bounty/hyperlane/

Detailed PoC Attached

A complete Foundry test suite demonstrating this vulnerability with multiple test cases is attached (see files below):

1. test_AccountingMismatch_Basic - Shows core vulnerability
2. test_VaultInsolvency_LPCannotWithdraw - Demonstrates fund lockup
3. test_AttackerMinimalCapital - Shows minimal attack surface
4. test_SharePriceManipulation - Shows share price inflation