c4-bot-10 commented 2 months ago

Lines of code

https://github.com/code-423n4/2024-06-krystal-defi/blob/f65b381b258290653fa638019a5a134c4ef90ba8/src/Common.sol#L668-L682

Vulnerability details

Impact

All main functionality risks reverting with tokens that revert on zero value transfers, via a transfer in Common._deductFees().

Proof of Concept

In Common._deductFees()

if (params.feeX64 == 0) {
    revert NoFees();
}

if (params.amount0 > 0) {
    feeAmount0 = FullMath.mulDiv(params.amount0, params.feeX64, Q64);
    amount0Left = params.amount0 - feeAmount0;
    SafeERC20.safeTransfer(IERC20(params.token0), FEE_TAKER, feeAmount0);
}
if (params.amount1 > 0) {
    feeAmount1 = FullMath.mulDiv(params.amount1, params.feeX64, Q64);
    amount1Left = params.amount1 - feeAmount1;
    SafeERC20.safeTransfer(IERC20(params.token1), FEE_TAKER, feeAmount1);
}
if (params.amount2 > 0) {
    feeAmount2 = FullMath.mulDiv(params.amount2, params.feeX64, Q64);
    amount2Left = params.amount2 - feeAmount2;
    SafeERC20.safeTransfer(IERC20(params.token2), FEE_TAKER, feeAmount2);
}

if 0 < params.feeX64 * params.amountX < Q64 then feeAmountX = 0. In this case the SafeERC20.safeTransfer() reverts on a token which reverts on zero transfers.

_deductFees() is used throughout the functionality of V3Automation and V3Utils.

Recommended Mitigation Steps

if (feeAmount0 > 0) {
    SafeERC20.safeTransfer(IERC20(params.token0), FEE_TAKER, feeAmount0);
}

etc.

Assessed type

ERC20

3docSec commented 2 months ago

Behavior in-scope as per README & report of sufficient quality

c4-judge commented 2 months ago

3docSec marked the issue as satisfactory