code-423n4 / 2022-02-concur-findings

2 stars 0 forks source link

`_perMille` bad assigment #235

Closed code423n4 closed 2 years ago

code423n4 commented 2 years ago

Lines of code

https://github.com/code-423n4/2022-02-concur/blob/main/contracts/MasterChef.sol#L57 https://github.com/code-423n4/2022-02-concur/blob/main/contracts/MasterChef.sol#L170-L175

Vulnerability details

Impact

MasterChef::_perMille is defined as 1000 and never changed. From the context, naming and comments, it is known that that it is to be representing a reciplocal of a basis point, which is 10000. This mistake will cause the depositFee to be 10x times larger than intended, which is cosidered a fund loss by a user.

Tools Used

Manual analysis

Recommended Mitigation Steps

Define _perMille as 10000. Additionally, it can be defined constant to save gas.

GalloDaSballo commented 2 years ago

The sponsor confirms the finding, I think Low Severity is more appropriate as the code doesn't reflect the intent. While you can definitely argue the sponsor made a mistake, ultimately integrators could just adapt

So I think Low Severity is more appropriate

JeeberC4 commented 2 years ago

Adding to warden's QA Report #241