search

code-423n4 / 2022-06-canto-findings

0 stars 0 forks source link

Accountant can't be initialized #53

Open code423n4 opened 2 years ago

code423n4 commented 2 years ago

Lines of code

https://github.com/Plex-Engineer/lending-market/blob/main/contracts/Accountant/AccountantDelegate.sol#L29

Vulnerability details

Impact

It's not possible to initialize the accountant because of a mistake in the function's require statement.

I rate it as MED since a key part of the protocol wouldn't be available until the contract is modified and redeployed.

Proof of Concept

The issue is the following require() statement: https://github.com/Plex-Engineer/lending-market/blob/main/contracts/Accountant/AccountantDelegate.sol#L29

There, the function checks whether the accountant has received the correct amount of tokens. But, it compares the accountant's balance with the _initialSupply. That value is always 0. So the require statement will always fail

When the Note contract is initialized, _initialSupply is set to 0:

After _mint_to_Accountant() mints type(uint).max tokens to the accountant: https://github.com/Plex-Engineer/lending-market/blob/main/contracts/Note.sol#L18 That increases the totalSupply but not the _initialSupply: https://github.com/Plex-Engineer/lending-market/blob/main/contracts/ERC20.sol#L242

The _initialSupply value is only modified by the ERC20 contract's constructor.

Tools Used

none

Recommended Mitigation Steps

Change the require statement to

require(note.balanceOf(msg.sender) == note.totalSupply(), "AccountantDelegate::initiatlize: Accountant has not received payment");
GalloDaSballo commented 2 years ago

The warden has shown how, due to an incorrect assumption, AccountantDelegate.initialize cannot work, meaning part of the protocol will never work without fixing this issue.

While the change should be fairly trivial, the impact is pretty high, for those reasons am going to raise severity to High