sherlock-audit / 2024-08-flayer-judging

2 stars 0 forks source link

Albort - Incorrect Handling of Token Denominations #746

Open sherlock-admin4 opened 1 month ago

sherlock-admin4 commented 1 month ago

Albort

High

Incorrect Handling of Token Denominations

Summary

The contract frequently multiplies token amounts by both 1 ether and 10 ** locker.collectionToken(...).denomination(). This double scaling can lead to exceedingly large values, especially if the token’s denomination is already set to 18 (a common standard for ERC20 tokens).

Vulnerability Detail

In createListings()

tokensReceived = _mapListings(listing, tokensIdsLength) * 10 ** locker.collectionToken(listing.collection).denomination();

Here, mapListings returns tokensReceived = _tokenIds 1 ether, which is already 1 10^18. Multiplying this by 10 * 18 (if denomination is 18) results in tokensReceived = _tokenIds 10^36, which is incorrect.

In cancelListings() uint requiredAmount = ((1 ether * _tokenIds.length) * 10 ** collectionToken.denomination()) - refund; Similar to the above, if _tokenIds.length = 1 and denomination = 18, requiredAmount becomes 10^36 - refund, which is not a valid ERC20 token amount.

Impact

Users might receive or be required to pay amounts that are astronomically high, effectively rendering the contract unusable for practical purposes.

Code Snippet

https://github.com/sherlock-audit/2024-08-flayer/blob/main/flayer/src/contracts/Listings.sol#L146

https://github.com/sherlock-audit/2024-08-flayer/blob/main/flayer/src/contracts/Listings.sol#L451

Tool used

Manual Review

Recommendation

Remove one layer of scaling. Typically, ERC20 tokens already account for their denomination, so using 1 ether (which is 10^18 wei) alongside 10 ** denomination is redundant. Instead, standardize the scaling based on the token’s denomination.