sherlock-audit / 2024-02-jala-swap-judging

6 stars 4 forks source link

santiellena - `ChilizWrappedERC20` is vulnerable to address collision #197

Closed sherlock-admin2 closed 7 months ago

sherlock-admin2 commented 8 months ago

santiellena

medium

ChilizWrappedERC20 is vulnerable to address collision

Summary

The factory ChilizWrapperFactory creates a new ChilizWrappedERC20 using CREATE2. A meet-in-the-middle attack at finding an address collision against an undeployed wrapped ERC20 is possible. Such attack could set attacker allowance of the underlying token to the max value and drain the wrapper.

Vulnerability Detail

Before going in details, this issue idea is from EIP-3607.

Past issues with a similar approach:

To carry out this attack, one must find a collision (or more) and then create a malicious smart contract that sets the attacker's wallet allowance to the maximum on the underlying token.

In ChilizWrapperFactory::_createWrappedToken the salt is the result of a common encoding of the underlying token address: bytes32 salt = keccak256(abi.encode(underlyingToken));

https://github.com/sherlock-audit/2024-02-jala-swap/blob/030d3ed54214754301154bce0e58ea534100a7e3/jalaswap-dex-contract/contracts/utils/ChilizWrapperFactory.sol#L34

assembly {
            wrappedToken := create2(0, add(bytecode, 32), mload(bytecode), salt)
        }

The attacker will need to find a collision of at least one of the many ERC20 Fan tokens (any token actually but as those are expected, risk is higher for them) that will be wrapped in this factory.

Then, the user will need to deploy its malicious smart contract with CREATE2, changing the salt value until a collision is found.

The following articles describe the feasibility of finding a collision:

At the time of this writing, there are 82 fan tokens available on Chiliz Chain and 1,332,095,893 USD of market cap. The current market data and the "low" cost of execution ( just a few million dollars), make this attack a really profitable strategy not only for existing tokens, but also for future new tokens.

It is then easy to show that such an attack is massively profitable.

This type of vulnerability has been extensively discussed in the two issues I mentioned in the beggining.

Impact

Draining of underlying assets from wrapped ERC20 contracts. Denial-of-Service of each JalaPair associated with these wrapped token.

Code Snippet

The idea behind this PoC was taken from this issue found in the Arcadia audit at Sherlock.

First,

Then,

All funds have been stolen and every JalaPair with a wrapped token from the factory has been DoSed.

Tool used

Manual Review

Recommendation

Do not use the underlying token as salt. Use the CREATE contract creation which relies on the address of the factory and its internal nonce. This will make the resulting address almost unpredictable so attempting a collision will be impractical.

To replace the ChilizWrapperFactory::wrappedTokenFor function use in JalaMasterRouter, I recommend using ChilizWrapperFactory::getUnderlyingToWrapped that has the practical same functionality.

nevillehuang commented 7 months ago

@Czar102 Given your new comments here, does that also apply to this issue?

Czar102 commented 7 months ago

It seems that aside for the extreme cryptographic difficulty of carrying out this attack, the attacker needs to track users to deposit funds into a pool collision they found – there are few pools for which the attacker could predict their existence and which would be used. I believe this is a low/info finding.