code-423n4 / 2021-07-sherlock-findings

0 stars 0 forks source link

Uncheckable math in `redeem()` #127

Open code423n4 opened 3 years ago

code423n4 commented 3 years ago

Handle

0xsanson

Vulnerability details

Impact

The equation ps.sherXUnderlying = ps.sherXUnderlying.sub(amounts[i]) can be calculated with "un-safe" math, since the previous operation (LibPool.payOffDebtAll(tokens[i])) makes sure that amounts[i] <= ps.sherXUnderlying.

Proof of Concept

https://github.com/code-423n4/2021-07-sherlock/blob/main/contracts/facets/SherX.sol#L286

Tools Used

editor

Recommended Mitigation Steps

Suggested write the equation as ps.sherXUnderlying -= amounts[i].

Evert0x commented 3 years ago

Fair point, not resolving the issue as the payOffDebtAll() function or code around it could be updated which can cause a change in behavior. Potentially creating an exploit if the 'un-safe' math is used.