code-423n4 / 2023-04-frankencoin-findings

5 stars 4 forks source link

Burn logic issue due to lack of checking parameter 0 in burnWithReserve function #986

Closed code423n4 closed 1 year ago

code423n4 commented 1 year ago

Lines of code

https://github.com/code-423n4/2023-04-frankencoin/blob/1022cb106919fba963a89205d3b90bf62543f68f/contracts/Frankencoin.sol#L234-L257

Vulnerability details

Impact

Proof of Concept

  1. burnWithReserve() -> _reservePPM = 0
    1. calculateFreedAmount() call -> The result is scaled by the ratio of currentReserve and minterReserve_ values through the constant operator, but since the final reservePPM value is 0, the adjustedReservePPM value will be 0. -> When the function returns, 1000000 * amountExcludingReserve / (1000000 - 0) = amountExcludingReserve -> Only the value of amountExcludingReserve can be used purely as is.
  2. it comes back and affects the sub-logic because the freedAmount value is 0.
  3. when _transfer is executed, freedAmount and _amountExcludingReserve are changed to 0 and transferred because they are the same value.
  4. finally, since the freedAmount value exists, it can be burned without sending the corresponding token.
  5. This is where the problem arises because the logic is done by returning the freedAmount value.

Tools Used

Static analysis

Recommended Mitigation Steps

c4-pre-sort commented 1 year ago

0xA5DF marked the issue as low quality report

0xA5DF commented 1 year ago

No PoC + not clear

it comes back and affects the sub-logic because the freedAmount value is 0.

No, the freed amount won't be zero in that case

c4-judge commented 1 year ago

hansfriese marked the issue as unsatisfactory: Invalid