Total supply can be incorrect in `ERC20`

[code423n4]

Lines of code

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

Vulnerability details

Impact

_totalSupply can be initialized to something different than 0, which would lead to an inaccurate total supply, and could easily break integrations, computations of market cap, etc.

Proof of Concept

If the constructor is called with _initialSupply = 1000, the totalSupply will be initialized to 1000.

As all the others computations are correct, there will be for ever a discrepancy of 1000 between the real total supply and the one of the contract.

Recommended Mitigation Steps

Remove _initialSupply.

[GalloDaSballo]

Same bug as from Canto V1. Recommend the sponsor to just set to 0 and remove the assignment from the constructor

[GalloDaSballo]

See: https://github.com/code-423n4/2022-06-canto-findings/issues/108

[Shungy]

In the provided contracts, v2 repo is included: https://github.com/code-423n4/2022-06-canto-v2

However, in this submission, the second line of code provided links to the v1 repo. The described issue only exists in v1 version. In v2 version the issue does not exist because msg.sender balance is updated along with the total supply: https://github.com/Plex-Engineer/lending-market-v2/blob/ea5840de72eab58bec837bb51986ac73712fcfde/contracts/ERC20.sol#L34

Therefore this finding seems invalid.

[GalloDaSballo]

In the provided contracts, v2 repo is included: https://github.com/code-423n4/2022-06-canto-v2

However, in this submission, the second line of code provided links to the v1 repo. The described issue only exists in v1 version. In v2 version the issue does not exist because msg.sender balance is updated along with the total supply: https://github.com/Plex-Engineer/lending-market-v2/blob/ea5840de72eab58bec837bb51986ac73712fcfde/contracts/ERC20.sol#L34

Therefore this finding seems invalid.

You're right, I must have missed the line with the mitigation.

The current code will update the _totalSupply and will give the balance to the deployer.

This is a mistake on my part and the finding should have been closed as invalid as it was mitigated in the V2 code in scope.

[GalloDaSballo]

While a nitpick I'd recommend changing the code to use _mint as it the code in scope will not emit an event which may cause issues if you're tracking via theGraph or similar

Either way I made a mistake here, sorry about that