Routing griefing via ERC-777 operator

[code423n4]

Lines of code

https://github.com/code-423n4/2023-04-caviar/blob/cd8a92667bcb6657f70657183769c244d04c015c/src/PrivatePool.sol#L459

Vulnerability details

Impact

Currently, there is no router implemented for private pools in which NFTs are traded against ERC-20 tokens (or it is not available in the repository). However, in the future, it is possible that some algorithm on the frontend will look for the optimal path for trading NFTs for traders.

A malicious pool owner can create multiple pools that are extremely beneficial for trading, in which NFTs will be traded against some ERC-777 token, such as the AMP token, which is traded on various popular exchanges:

• https://etherscan.io/token/0xff20817765cb7f73d4bde2e66e067e58d11095c2

Then, the front-end algorithm will select paths for trading that involve these pools.

The malicious pool owner can use the PrivatePool.execute() function to set themselves as the ERC-777 token operator on behalf of the pool in which this token is used for trading against NFTs.

ERC-777 tokens allow setting hooks that are executed when funds are transferred. A hacker can set their own hook, which will be executed every time the address of this hacker is the sender or recipient of funds. For most ERC-777 tokens, the hook is set through the global registry ERC1820Registry.setInterfaceImplementer(imlementer=callbackAddress):

• https://etherscan.io/address/0x1820a4B7618BdE71Dce8cdc73aAB6C95905faD24

The malicious pool owner can call PrivatePool.execute() and call ERC1820Registry.setInterfaceImplementer(imlementer=callbackAddress) on behalf of the pool. After that, every time funds are transferred into or out of the pool, the callbackAddress of the pool owner will be executed.

The owner can set a hook that will call revert() every time someone tries to deposit funds into the pool. The profitable paths suggested by the algorithm on the frontend will no longer work.

This will hinder the work of regular traders with the frontend, meaning that it's essentially a denial-of-service (DoS) attack.

Recommended Mitigation Steps

It is recommended to either remove the execute() function or prohibit calling ERC1820Registry from the execute() function:

require(target != 0x1820a4B7618BdE71Dce8cdc73aAB6C95905faD24);

[c4-pre-sort]

0xSorryNotSorry marked the issue as primary issue

[outdoteth]

We don't intend to support ERC-777 tokens. This was indicated in discord:

griefing attack also does not lead to any direct loss of funds.

[c4-sponsor]

outdoteth marked the issue as sponsor acknowledged

[c4-sponsor]

outdoteth marked the issue as disagree with severity

[c4-judge]

GalloDaSballo marked the issue as unsatisfactory: Invalid

[GalloDaSballo]

Overly inflated imo, maybe valid as QA