code-423n4 / 2022-12-tigris-findings

8 stars 4 forks source link

Assuming that tokens will always be 18 decimal #611

Closed code423n4 closed 1 year ago

code423n4 commented 1 year ago

Lines of code

https://github.com/code-423n4/2022-12-tigris/blob/main/contracts/BondNFT.sol#L180

Vulnerability details

Impact

The contract is always divided by 10**18 assuming that all tokens will be 18 decimal but there is nothing that force an ERC20 token to be 18 decimal. Since there is function addAsset then this means that different ERC20 token can be added and one of them may be a token with 18 decimal.

Proof of Concept

Any token with less or more than 18 decimal will cause a wrong calculation in different places in the contract. https://github.com/code-423n4/2022-12-tigris/blob/main/contracts/BondNFT.sol#L178

Tools Used

VSCODE

Recommended Mitigation Steps

Get the decimals from the function decimals() and divide by it.

GalloDaSballo commented 1 year ago

Looks invalid, but will flag to sponsor

The 1e18 are not decimals, but rather a precision factor added here: https://github.com/code-423n4/2022-12-tigris/blob/588c84b7bb354d20cbca6034544c4faa46e6a80e/contracts/BondNFT.sol#L225

TriHaz commented 1 year ago

@GalloDaSballo is right.

c4-sponsor commented 1 year ago

TriHaz marked the issue as sponsor disputed

GalloDaSballo commented 1 year ago

Per the sponsor confirmation, am closing as invalid

c4-judge commented 1 year ago

GalloDaSballo marked the issue as unsatisfactory: Invalid