code-423n4 / 2022-01-trader-joe-findings

2 stars 0 forks source link

Unsafe call to decimals() #291

Open code423n4 opened 2 years ago

code423n4 commented 2 years ago

Handle

pauliax

Vulnerability details

Impact

This is an issue as well as a gas improvment. Create pair adjusts token amounts based on floor price:

    if (
        floorPrice > (wavaxReserve * 10**token.decimals()) / tokenAllocated
    ) {
        tokenAllocated = (wavaxReserve * 10**token.decimals()) / floorPrice;

The problem is that .decimals() is not part of the official ERC20 standard and might fail for tokens that do not implement it. While in practice it is very unlikely, as usually most of the tokens implement it, this should still be considered as a valid concern, especially considering that createPair can only be called once after phase 3 has started.

Recommended Mitigation Steps

The current approach may be useful in case the token can change its decimals (usually very unlikely but possible), but I think a better solution would be to initialize the decimals in the initialize function or at least make a test call to see if this token is compatible. Another possibility is to write a helper function, e.g. similar to the popular safeTransfer.

And finally, a gas improvement here would be to avoid this duplicate external call and multiplication: (wavaxReserve * 10**token.decimals()) You should cache the result and re-use it inside the if statement.