Lucene search

K
code423n4Code4renaCODE423N4:2022-08-MIMO-FINDINGS-ISSUES-153
HistoryAug 07, 2022 - 12:00 a.m.

[H3] Persisted msg.value in a loop of delegate calls can be used to drain ETH from your proxy

2022-08-0700:00:00
Code4rena
github.com
4
vulnerability
delegate call
proxy funds

Lines of code

Vulnerability details

Impact

msg.value in a loop can be used to drain proxy funds

PoC

While BoringBatchable is out of the scope, this bug affects seriously MIMOProxy as it inherits.

Some time ago I read a report about an auditor called samczsung (<https://samczsun.com/two-rights-might-make-a-wrong/&gt;). I believe that you are having the same problem here.

I will try to explain it as brief as possible but I can add a PoC in QA stage if required.

First step: Draining ETH

This vulnerability comes from the fact that msg.value and msg.sender are persisted in delegatecall.

It is possible to call execute() (which is payable ) from batch() (which is also payable ) because both are public functions. (For now ignore the fact that execute() has access control).

The attacker would call batch() sending, for example, 1 ETH with an array of 100 equal items that call execute()

This execute() will call and external contract 100 times and in every time it will send 1ETH from proxy funds (not from the attacker).

If the receiving contract stores these value then the proxy wallet will be drained.

Second step: Access control bypass scenario

While this is already a high risk and there should be many attacking scenarios I would like to show you a pretty simple one.

Suppose the owner would like to grant access to a target with a normal function (maybe no even payable).

For example suppose that the owner grant access to the function

  function goodFunction() public 

This function has the selector 0x0d092393 . However, for some reason. the owner mistyped the selector and grant access to non existing function 0x0d09392.

Then if the target contract has the so common function.

fallback() external payable {  }

Then the attacker can drain wallet funds using this selector as I explained above.

Recommended

The solution is pretty straightforward.

Remove payable from batch() in BoringBatchable.


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

šŸ‘€ 1 horsefacts reacted with eyes emoji

All reactions

  • šŸ‘€ 1 reaction