code-423n4 / 2023-08-goodentry-findings

3 stars 2 forks source link

`executeOperation` in `OptionsPositionManager` lacks `initiator` check #434

Closed code423n4 closed 1 year ago

code423n4 commented 1 year ago

Lines of code

https://github.com/code-423n4/2023-08-goodentry/blob/main/contracts/PositionManager/OptionsPositionManager.sol#L42

Vulnerability details

Impact

Anyone can buy options on behalf of other users.

Proof of Concept

  function executeOperation(
    address[] calldata assets,
    uint256[] calldata amounts,
    uint256[] calldata premiums,
    address initiator,
    bytes calldata params
  ) override external returns (bool result) {
    uint8 mode = abi.decode(params, (uint8) );
    // Buy options
    if ( mode == 0 ){
      (, uint poolId, address user, address[] memory sourceSwap) = abi.decode(params, (uint8, uint, address, address[]));
      executeBuyOptions(poolId, assets, amounts, user, sourceSwap);
    }

Aave's flashloan allows to pass a receiverAddress when calling flashloan. This address will be called by aave to invoke the flashloan callback. The attacker can deploy a malicious to call flashloan and set receiverAddress to OptionsPositionManager.

  function flashLoan(
    address receiverAddress,
    address[] calldata assets,
 ....

The flashloan will call OptionsPositionManager's executeOperation function.

The attacker can pass a constructed params that sets the executeBuyOptions parameters to buy options on behalf of any user.

Tools Used

Manual review

Recommended Mitigation Steps

Add a check on initiator.

  function executeOperation(
    address[] calldata assets,
    uint256[] calldata amounts,
    uint256[] calldata premiums,
    address initiator,
    bytes calldata params
  ) override external returns (bool result) {
    uint8 mode = abi.decode(params, (uint8) );
    // Buy options
    if ( mode == 0 ){
      (, uint poolId, address user, address[] memory sourceSwap) = abi.decode(params, (uint8, uint, address, address[]));
+     require(initiator == user);
      executeBuyOptions(poolId, assets, amounts, user, sourceSwap);
    }

Assessed type

Access Control

c4-pre-sort commented 1 year ago

141345 marked the issue as duplicate of #110

c4-judge commented 1 year ago

gzeon-c4 marked the issue as unsatisfactory: Invalid