code-423n4 / 2021-09-wildcredit-findings

0 stars 0 forks source link

Remove pair-specific parameters until they are actually used/enforced #65

Open code423n4 opened 3 years ago

code423n4 commented 3 years ago

Handle

0xRajeev

Vulnerability details

Impact

As per the code comment “InterestRateModel can later be replaced for more granular fees per _pair”, the various functions in this contract are meant to be pair-specific but are currently not. However, three functions already accept parameters (which are currently unused) to enable this in future. This makes the current code hard to read and also uses more gas because of all the unused parameters/arguments.

Proof of Concept

https://github.com/code-423n4/2021-09-wildcredit/blob/c48235289a25b2134bb16530185483e8c85507f8/contracts/InterestRateModel.sol#L66-L85

https://github.com/code-423n4/2021-09-wildcredit/blob/c48235289a25b2134bb16530185483e8c85507f8/contracts/InterestRateModel.sol#L87-L96

https://github.com/code-423n4/2021-09-wildcredit/blob/c48235289a25b2134bb16530185483e8c85507f8/contracts/InterestRateModel.sol#L98-L101

Tools Used

Manual Analysis

Recommended Mitigation Steps

Remove pair-specific parameters until these contract functions actually are made granular. This will require a redeployment because there is no proxy-upgradeable pattern used here. This will unnecessarily consume gas and also give the impression that these functions are already pair-specific.

talegift commented 3 years ago

As the report already states:

This will require a redeployment because there is no proxy-upgradeable pattern used here

It's not possible to temporarily remove the arguments and bring them back later. It's also not a bug. There is zero risk here.

ghoul-sol commented 3 years ago

This is a recommendation. Non-critical.