Lines of code
<https://github.com/code-423n4/2022-08-mimo/tree/main/contracts/proxy/MIMOProxy.sol#L104&gt;

Vulnerability details

Impact

• The multicall doesn’t check if the owner has changed after call or calls has been made. The transferOwnerShip() contracts/proxy/MIMOProxy.sol/ requires that the owner is the msg.sender, before ownership can be changes, which is exactly what multicall can do, same with setPermissions. Auths should be checked before and after calls(delegate and statics too)

Affected lines of code

• <https://github.com/code-423n4/2022-08-mimo/tree/main/contracts/proxy/MIMOProxy.sol#L127&gt;
• <https://github.com/code-423n4/2022-08-mimo/tree/main/contracts/proxy/MIMOProxy.sol#L104&gt;

Proof of Concept

• on line 119, transferOwnerShip() contracts/proxy/MIMOProxy.sol/#L119 requires that the owner is the msg.sender to change the owner,

  ──────-┬──────────────────────────────────────────────────────────────────────
  │ File: contracts/proxy/MIMOProxy.sol
  ───────┼──────────────────────────────────────────────────────────────-------
  117 │ function transferOwnership(address newOwner) external override {
  118 │ address oldOwner = owner;
  119 │ if (oldOwner != msg.sender) {
  120 │ revert CustomErrors.NOT_OWNER(oldOwner, msg.sender);
  121 │ }
  122 │ owner = newOwner;
  123 │ emit TransferOwnership(oldOwner, newOwner);
  124 │ }

• an external call made to an arbitrary contract call made through multicall can make MIMOProxy call itself, be the msg.sender and be able to change owner

• on line 104 too, setPermission() contracts/proxy/MIMOProxy.sol/#L104 requires that the owner is the msg.sender to set and change permissions, but an external call made through multicall can make the MIMOProxy call itself and setPermissions through that

  ───────┬─────────────────────────────────────────────────────----------
  │ File: contracts/proxy/MIMOProxy.sol
  ───────┼────────────────────────────────────────────────────────────────
  104 │ function setPermission(
  105 │ address envoy,
  106 │ address target,
  107 │ bytes4 selector,
  108 │ bool permission
  109 │ ) public override {
  110 │ if (owner != msg.sender) {
  111 │ revert CustomErrors.NOT_OWNER(owner, msg.sender);
  112 │ }
  113 │ _permissions[envoy][target][selector] = permission;
  114 │ }

Tools Used

vim, foundry

Recommended Mitigation Steps

• check to make sure the owner didn’t change after multicall, by storing it in a local variable and making sure it didn’t change after the call or calls has been made

  ───────┬─────────────────────────────────────────────────────────────────────────
  │ File: contracts/proxy/MIMOProxy.sol
  ───────┼─────────────────────────────────────────────────────────────────────────
  127 │ function multicall(address[] calldata targets, bytes[] calldata data) external override returns (bytes[] memory) {
  128 │ if (msg.sender != owner) {
  129 │ revert CustomErrors.NOT_OWNER(owner, msg.sender);
  130 │ }

  •     address _owner = owner;
    

  131 │ bytes[] memory results = new bytes;
  132 │ for (uint256 i = 0; i < targets.length; i++) {
  133 │ (bool success, bytes memory response) = targets[i].call(data[i]);
  134 │ if (!success) {
  135 │ if (response.length > 0) {
  136 │ assembly {
  137 │ let returndata_size := mload(response)
  138 │ revert(add(32, response), returndata_size)
  139 │ }
  140 │ } else {
  141 │ revert CustomErrors.LOW_LEVEL_CALL_FAILED();
  142 │ }
  143 │ }
  144 │ results[i] = response;
  145 │ }

  •    if (_owner != owner) revert CustomErrors.NOT_OWNER(oldOwner, msg.sender);   
    

  146 │ return results;
  147 │ }
  148 │ }

The text was updated successfully, but these errors were encountered:

All reactions