In ERC20, TotalSupply is broken

[code423n4]

Lines of code

https://github.com/Plex-Engineer/lending-market/blob/ab31a612be354e252d72faead63d86b844172761/contracts/ERC20.sol#L33
https://github.com/Plex-Engineer/lending-market/blob/ab31a612be354e252d72faead63d86b844172761/contracts/ERC20.sol#L95

Vulnerability details

Impact

For an obscure reason as it’s not commented, _totalSupply is not initialized to 0, leading to an inaccurate total supply, which could easily break integrations, computations of market cap, etc.

Proof of Concept

If the constructor is called with _initialSupply = 1000, then 1000 tokens are minted. The total supply will be 2000.

Recommended Mitigation Steps

Remove _initialSupply.

[tkkwon1998]

The explanation is not clear. We can't seem to reproduce this issue as we can't find a scenario where the totalSupply function returns an incorrect value.

[Picodes]

@tkkwon1998 to clarify:

Deploy the ERC20 with totalSupply_ = 1000.

Then totalSupply() returns 1000, which is incorrect.

Then if someone mints 1000 tokens, there is 1000 tokens in the market but due to _totalSupply += amount;, totalSupply = 2000 which is still incorrect

[GalloDaSballo]

I believe the submission could have benefitted by:

• A coded POC
• Recognizing a revert due to the finding

However the finding is ultimately true in that, because totalSupply is a parameter passed in to the contract, and the ERC20 contract will not mint that amount, the totalSupply will end up not reflecting the total amounts of tokens minted.

For this reason, I believe the finding to be valid and High Severity to be appropriate.

I recommend the warden to err on the side of giving too much information to avoid getting their finding invalidated incorrectly

[GalloDaSballo]

After further thinking, I still believe the finding is of high severity as the ERC20 standard is also broken, I do believe the submission could have been better developed, however, I think High is in place here