code-423n4 / 2023-05-caviar-mitigation-contest-findings

0 stars 0 forks source link

Mitigation Confirmed for H-03 #20

Open code423n4 opened 1 year ago

code423n4 commented 1 year ago

Mitigation of H-03: Issue mitigated

Issue

H-03: Risk of silent overflow in reserves update

Mitigation

https://github.com/outdoteth/caviar-private-pools/pull/10

Assessment of Mitigation

Issue mitigated

Comments

Openzeppelin's SafeCast.toUint128 function below is now used to downcast the corresponding uint256 values to uint128 as follows.

https://github.com/outdoteth/caviar-private-pools/pull/10/files#diff-6beea546a01e795e1365ca8a74b2bd952631f6cd228a5a869bfed82bf1f46a0fR232-R233

        virtualBaseTokenReserves += SafeCast.toUint128(netInputAmount - feeAmount - protocolFeeAmount);
        virtualNftReserves -= SafeCast.toUint128(weightSum);

https://github.com/outdoteth/caviar-private-pools/pull/10/files#diff-6beea546a01e795e1365ca8a74b2bd952631f6cd228a5a869bfed82bf1f46a0fR325-R326

        virtualBaseTokenReserves -= SafeCast.toUint128(netOutputAmount + protocolFeeAmount + feeAmount);
        virtualNftReserves += SafeCast.toUint128(weightSum);

Because of require(value <= type(uint128).max, "SafeCast: value doesn't fit in 128 bits") in Openzeppelin's SafeCast.toUint128 function, downcasting netInputAmount - feeAmount - protocolFeeAmount, weightSum, and netOutputAmount + protocolFeeAmount + feeAmount to uint128 would revert if they are bigger than type(uint128).max. Hence, the corresponding issue is mitigated.

https://github.com/OpenZeppelin/openzeppelin-contracts/blob/master/contracts/utils/math/SafeCast.sol#L290-L293

    function toUint128(uint256 value) internal pure returns (uint128) {
        require(value <= type(uint128).max, "SafeCast: value doesn't fit in 128 bits");
        return uint128(value);
    }
c4-judge commented 1 year ago

GalloDaSballo marked the issue as satisfactory