code-423n4 / 2023-09-goodentry-mitigation-findings

0 stars 0 forks source link

Use of tx.origin breaks interoperability with AA wallets. #58

Closed c4-submissions closed 12 months ago

c4-submissions commented 1 year ago

Lines of code

https://github.com/GoodEntry-io/GoodEntryMarkets/blob/2e3d23016fadb45e188716d772cec7c2096fae01/contracts/protocol/lendingpool/LendingPool.sol.0x20#L492

Vulnerability details

In OptionPositionMananger, several functions like close and sellOptions, need to call PMWithdraw, which calls PMTransfer. Then it is checked that tx.origin != user. However, smart contract wallet cannot be tx.origin, which means AA wallets will not be able to interact with the protocol.

  function PMTransfer(
    address aAsset,
    address user,
    uint256 amount
  ) external whenNotPaused {
    require(pm[msg.sender], "Not PM");
    if (tx.origin != user) {
      (,,,, uint256 healthFactor) = GenericLogic.calculateUserAccountData(
        user,
        _reserves,
        _usersConfig[user],
        _reservesList,
        _reservesCount,
        _addressesProvider.getPriceOracle()
        );
      require(healthFactor <= softLiquidationThreshold, "Not initiated by user");
    }
    IAToken(aAsset).transferOnLiquidation(user, msg.sender, amount);
  }

I suggest we move the check to OptionPositionMananger and check msg.sender == user instead.

Assessed type

Context

c4-judge commented 12 months ago

gzeon-c4 marked the issue as duplicate of #17

c4-judge commented 12 months ago

gzeon-c4 marked the issue as satisfactory