code-423n4 / 2022-04-phuture-findings

0 stars 0 forks source link

Burn function doesn't burn the correct value #51

Closed code423n4 closed 2 years ago

code423n4 commented 2 years ago

Lines of code

https://github.com/code-423n4/2022-04-phuture/blob/594459d0865fb6603ba388b53f3f01648f5bb6fb/contracts/libraries/NAV.sol#L60

Vulnerability details

Impact

Burn function in line 60 in NAV contract burn value instead of amount

Proof of Concept

https://github.com/code-423n4/2022-04-phuture/blob/594459d0865fb6603ba388b53f3f01648f5bb6fb/contracts/libraries/NAV.sol#L60 you can see there.

Tools Used

Recommended Mitigation Steps

change the line from

        _burn(self, address(this), value);

to

        _burn(self, address(this), amount);
jn-lp commented 2 years ago

We expect value to be burned, not amount

moose-code commented 2 years ago

Agree with sponsor, can see value is expected. Note it is very confusing to have names like value and amount which sound identical without more information. Recommend more descriptive variable names to avoid this confusion. valueOfShares amountOfAssets etc...