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

2 stars 2 forks source link

`AMM._getPnlWhileReducingPosition()` Does Not Handle The Zero Case #136

Closed code423n4 closed 2 years ago

code423n4 commented 2 years ago

Lines of code

https://github.com/code-423n4/2022-02-hubble/blob/main/contracts/AMM.sol#L264 https://github.com/code-423n4/2022-02-hubble/blob/main/contracts/AMM.sol#L554-L557

Vulnerability details

Impact

_getPnlWhileReducingPosition() is called by the removeLiquidity() function to perform accounting on the change in position. However, the function does not correctly handle the edge case where totalPosition is zero. As a result, the function will revert as it attempts to divide by zero, preventing the removal of liquidity.

Proof of Concept

Tools Used

Manual code review.

Recommended Mitigation Steps

Consider checking this edge case as per the @todo statement in the _getPnlWhileReducingPosition() function.

atvanguard commented 2 years ago

Known issue that is already marked as a todo in the referenced code lines.

moose-code commented 2 years ago

In this case I am going to side with the sponsor and this is very clearly stated in the Todo. In some other submissions, the todo is more cryptic, and a detailed analysis of the issue is valid.

JeeberC4 commented 2 years ago

Grouping with warden's QA Report #131