Lines of code

Vulnerability details

Fallback handlers can trick users into calling functions of the AmbireAccount contract

Selector clashing can be used to trick users into calling base functions of the wallet.

Impact

Fallback handlers provide extensibility to the Ambire wallet. The main idea here is that functions not present in the wallet implementation are delegated to the fallback handler by using the fallback() function.

Function dispatch in Solidity is done using function selectors. Selectors are represented by the first 4 bytes of the keccak hash of the function signature (name + argument types). It is possible (and not computationally difficult) to find different functions that have the same selector.

This means that a malicious actor can craft a fallback handler with a function signature carefully selected to match one of the functions present in the base AmbireAccount contract, and with an innocent looking implementation. While the fallback implementation may seem harmless, this function when called will actually trigger the function in the base AmbireAccount contract. This can be used, for example, to hide a call to setAddrPrivilege() which could be used to grant control of the wallet to the malicious actor.

This is similar to the exploit reported on proxies in this article, which caused the proposal of the transparent proxy pattern.

As further reference, another similar issue can be found in the DebtDAO contest that could lead to unnoticed calls due to selector clashing (disclaimer: the linked report is authored by me).

Recommendation

It is difficult to provide a recommendation based on the current design of contracts. Any whitelisting or validation around the selector won’t work as the main entrypoint of the wallet is the AmbireAccount contract itself. The solution would need to be based on something similar to what was proposed for transparent proxies, which involves segmenting the calls to avoid clashing, but this could cripple the functionality and simplicity of the wallet.

Assessed type

Other

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

All reactions