Should spend allowance from msg.sender rather than from borrower.

Summary

Spending allowance from borrower/owner when calling the borrow, borrowAtMaturity and withdrawAtMaturity opens them up to having their allowance spent by malicious users.

Vulnerability Detail

When the borrow function is called, the msg.sender has the option of passing in any borrower address of their choice. While, this is to service the MarketETHRouter, this can pose a risk to users who have any unspent allowance in the market contract, or max allowance. Malicious users can borrow assets in the victims name which saddles them unwanted debt, while not receiving the borrowed assets.

  function borrow(
    uint256 assets,
    address receiver,
    address borrower
  ) external whenNotPaused whenNotFrozen returns (uint256 borrowShares) {
    spendAllowance(borrower, assets);
...
    asset.safeTransfer(receiver, assets);
  }

The same can be observed in borrowAtMaturity and withdrawAtMaturity functions.

Impact

Users with unspent allowance in the Market contract can have assets borrowed in their name, saddling them with debts and leading to loss of funds.

Code Snippet

https://github.com/sherlock-audit/2024-04-interest-rate-model/blob/8f6ef1b0868d3ea3a98a5ab7e8b3a164857681d7/protocol/contracts/Market.sol#L145 https://github.com/sherlock-audit/2024-04-interest-rate-model/blob/8f6ef1b0868d3ea3a98a5ab7e8b3a164857681d7/protocol/contracts/Market.sol#L327 https://github.com/sherlock-audit/2024-04-interest-rate-model/blob/8f6ef1b0868d3ea3a98a5ab7e8b3a164857681d7/protocol/contracts/Market.sol#L411

Tool used

Manual Code Review

Recommendation

Spend allowance from msg.sender instead.

  function borrow(
    uint256 assets,
    address receiver,
    address borrower
  ) external whenNotPaused whenNotFrozen returns (uint256 borrowShares) {
    spendAllowance(msg.sender, assets);
...
    asset.safeTransfer(receiver, assets);
  }

To handle the MarketETHRouter, allow users to transfer assets to it instead, then approve Market to spend the assets.

  function borrow(uint256 assets) external unwrap(assets) returns (uint256 borrowShares) {
    asset.safeTransferFrom(msg.sender, address(this), assets)
    asset.safeApprove(Market, assets);
    return market.borrow(assets, address(this), msg.sender);
  }

sherlock-audit / 2024-04-interest-rate-model-judging