When _reserveDelta is negative in ABC._calculateDeltaOfFSD the following branch is executed:
if (_reserveDelta < 0) {
uint256 capitalPostWithdrawal =
capitalPool.sub(uint256(_reserveDelta));
The type cast to uint256 is purely a reinterpretation of the underlying bytes, it does not compute the absolute value.
Which means uint256(_reserveDelta) will be a huge value (2^256 - abs(_reserveDelta)) due to two's complement encoding. This is always greater than the capitalPool and the transaction will revert because of the underflow in SafeMath.sub.
Impact
The FSD.burn function always calls it with a negative value which would break the burn function among other functions. (Why is this not caught in a test though?)
Recommendation
Multiply _reserveDelta by -1 to get the absolute value and then the type-cast to uint256 will be safe.
Handle
cmichel
Vulnerability details
Vulnerability Details
When
_reserveDelta
is negative inABC._calculateDeltaOfFSD
the following branch is executed:The type cast to
uint256
is purely a reinterpretation of the underlying bytes, it does not compute the absolute value. Which meansuint256(_reserveDelta)
will be a huge value (2^256 - abs(_reserveDelta)
) due to two's complement encoding. This is always greater than thecapitalPool
and the transaction will revert because of the underflow inSafeMath.sub
.Impact
The
FSD.burn
function always calls it with a negative value which would break theburn
function among other functions. (Why is this not caught in a test though?)Recommendation
Multiply
_reserveDelta
by-1
to get the absolute value and then the type-cast touint256
will be safe.