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

0 stars 0 forks source link

If statement should output ps.protoclBalance if amount > ps.protocolBalance #128

Closed code423n4 closed 3 years ago

code423n4 commented 3 years ago

Handle

tensors

Vulnerability details

Impact

In L287 of PoolBase.sol an if statement checks whether to give the full balance of the protocol. This happens if uint(-1) is the input amount. It would be better if this happened whenever amount > balance of the protocol. Saving on gas fees if there was a misinput by mistake.

Proof of Concept

https://github.com/code-423n4/2021-07-sherlock/blob/d9c610d2c3e98a412164160a787566818debeae4/contracts/facets/PoolBase.sol#L287

Recommended Mitigation Steps

Modify the function slightly to check if _amount > ps.protocolBalance.

Evert0x commented 3 years ago

This could cause unexpected behavior / confusion if an account withdraws more then ps.protocolBalance, but expects it to be less or equal to ps.protocolBalance.

The interacting code will move on expecting to have the _amount withdrawn instead of the < leftover balance.

ghoul-sol commented 3 years ago

using uint(-1) as max withdraw has been a silent best practice. Agree with sponsor, invalid