code-423n4 / 2021-07-sherlock-findings

0 stars 0 forks source link

`TokenToLock` default value #110

Open code423n4 opened 3 years ago

code423n4 commented 3 years ago

Handle

cmichel

Vulnerability details

The PoolBase.TokenToLockXRate function returns the "Current exchange rate from _token to lockToken".

It does not specify the precision and according to the documentation, it sounds like one just has to multiply this value by any _token amount to get the corresponding lockToken amount. Lock tokens are always in 18 decimals (as is their initial mint) but _token does not have to be. The return value TokenToLock(10**18, _token) = totalLock * 1e18 / balance has the wrong precision, it should return TokenToLock(10**(_token.decimals()), _token) instead returning the lock amount per "1.0" _token.

Evert0x commented 3 years ago

18 Decicmals

If both Token (T) and LockToken (LT) are 18 decimals

and 1 LT = 10 T meaning 1 10*18 = 10 * 10**18

Which means 100 LT wil be 1000 T

100 (18 decimals) * LTxT / 10**18 = 1000 (18 decimals)

Which means 1000 T will be 100 LT

1000 (18 decimals) * TxLT / 10**18 = 100 (18 decimals)

So far so good, this was an example with an 18 decimal token, but afaik, the same logic applies to a 6 (or whatever) decimal token.

6 Decimals

If T is 6 decimals and LT 18 decimals

and 1 LT = 10 T meaning 1 10*18 = 10 * 10**6

Which means 100 LT wil be 1000 T

100 (18 decimals) * LTxT / 10**18 = 1000 (6 decimals)

Which means 1000 T will be 100 LT

1000 (6 decimals) * TxLT / 10**18 = 100 (18 decimals)

Evert0x commented 3 years ago

Added this in a test btw, https://github.com/sherlock-protocol/sherlock-v1-core/blob/main/test/Pool.js#L844

Evert0x commented 3 years ago

After discussing this finding with the warden it was concluded this is a non-issue.