AuditContest-11-25

AuditContest-11-25(Updating)

Alchemix V3

Silent Failure for Non-standard ERC20 Transfers in deposit()/withdraw()

Severity: Insight

Context: AlchemistTokenVault.sol#L28 && L41

Summary:

AlchemistTokenVault.deposit() and withdraw() call the raw ERC‑20 transferFrom/transfer and drop the boolean return value. For tokens that signal failure by returning false (USDT-style implementations), the functions still emit Deposited/Withdrawn even though no funds move, so the vault state diverges from reality.

Vulnerability Detail:

For some old token like USDT, transfer/transferFrom return a bool to express success/fail.

If the balance is insufficient, the credit limit is insufficient, the address is on the blacklist, etc.

in deposit/withdraw:

// deposit
IERC20(token).transferFrom(msg.sender, address(this), amount);
emit Deposited(msg.sender, amount);

// withdraw
IERC20(token).transfer(recipient, amount);
emit Withdrawn(recipient, amount);

This is safe only for tokens that revert on failure. Many legacy or heavily permissioned tokens (e.g., USDT) follow the original ERC‑20 convention: they return false when a transfer fails—due to insufficient balance, allowance, a freeze/blacklist flag, etc.—and do not revert. In those cases the vault still emits Deposited/Withdrawn, updates whatever internal bookkeeping comes afterward, but no funds actually move. As a result:

• Deposits that should fail appear to succeed, leaving the vault with less collateral than it believes.
• Withdrawals silently fail, so fee recipients or liquidators never receive the tokens they are owed, yet the system records the payout.

Imapct: Because the protocol continues operating on bogus accounting and cannot deliver funds to participants, this constitutes a high‑severity silent failure.

Proof of Concept:

contract MockNonCompliantToken is ERC20 {
    bool public failTransfers;

    constructor() ERC20("Mock Non Compliant", "MNCT") {
        _mint(msg.sender, 1_000_000 * 10 ** 18);
    }

    function setFailTransfers(bool status) external {
        failTransfers = status;
    }

    function transfer(address to, uint256 amount) public override returns (bool) {
        if (failTransfers) {
            return false;
        }
        return super.transfer(to, amount);
    }

    function transferFrom(address from, address to, uint256 amount) public override returns (bool) {
        if (failTransfers) {
            return false;
        }
        return super.transferFrom(from, to, amount);
    }
}

function testNonStandardTokenCausesSilentDepositFailure() public {
        MockNonCompliantToken faultyToken = new MockNonCompliantToken();
        address depositor = address(42);

        // Move funds to depositor while transfers work
        faultyToken.transfer(depositor, AMOUNT);

        // Deploy a fresh vault that manages the faulty token
        vm.prank(owner);
        AlchemistTokenVault faultyVault = new AlchemistTokenVault(address(faultyToken), alchemist, owner);

        // Allow the vault to pull funds from depositor
        vm.prank(depositor);
        faultyToken.approve(address(faultyVault), AMOUNT);

        // Flip the token into "non-standard" mode (returns false on transfer)
        faultyToken.setFailTransfers(true);

        // Deposit proceeds without reverting, but funds never move (silent failure)
        vm.prank(depositor);
        faultyVault.deposit(AMOUNT);

        assertEq(faultyToken.balanceOf(address(faultyVault)), 0, "Vault should still be empty");
        assertEq(faultyToken.balanceOf(depositor), AMOUNT, "Depositor should retain tokens");

        // Downstream withdraw also appears successful while transferring nothing.
        address recipient = address(99);
        uint256 preRecipient = faultyToken.balanceOf(recipient);

        vm.prank(alchemist);
        faultyVault.withdraw(recipient, AMOUNT);

        assertEq(faultyToken.balanceOf(address(faultyVault)), 0, "Vault stays empty after withdraw");
        assertEq(faultyToken.balanceOf(recipient), preRecipient, "Recipient receives nothing despite success");
    }

Recommendation:

Use the safe wrappers from OpenZeppelin so that any ERC‑20 returning false triggers an immediate revert. In practice:

import {SafeERC20} from "@openzeppelin/contracts/token/ERC20/utils/SafeERC20.sol";

using SafeERC20 for IERC20;

function deposit(uint256 amount) external {
    _checkNonZeroAmount(amount);
    IERC20(token).safeTransferFrom(msg.sender, address(this), amount);
    emit Deposited(msg.sender, amount);
}

function withdraw(address recipient, uint256 amount) external override onlyAuthorized {
    _checkNonZeroAddress(recipient);
    _checkNonZeroAmount(amount);
    IERC20(token).safeTransfer(recipient, amount);
    emit Withdrawn(recipient, amount);
}

_mytSharesDeposited didn’t get updated after _forceRepay && _doLiquidation called

Severity: High

Context: AlchemistV3.sol#L738 && L791

Summary:

In AlchemistV3.sol, the _forceRepay and _doLiquidation transfer MYT shares out of the contract (to the transmuter, liquidator, or protocol fee vault) but never decrement _mytSharesDeposited. Consequently, the internal ledger continues to assume those shares are still held. This drifts collateral accounting—global collateralization metrics are overstated, so liquidations can under-react—and deposit-cap enforcement rejects new deposits even after MYT has actually left the vault.

Vulnerability Detail:

Within _forceRepay and _doLiquidation, the contract transfers MYT shares to external recipients(typically the transmuter, liquidator, or protocol fee vault), but _mytSharesDeposited is never reduced by the amount that actually left the contract. Subsequent logic such as _getTotalUnderlyingValue(), calculateLiquidation, and depositCap checks rely on _mytSharesDeposited as the canonical measure of MYT held by AlchemistV3.

Because the counter is not updated after these transfers, every invocation of _forceRepay or _doLiquidation causes the internal ledger to drift further from the real balance. This bug triggers on every liquidation or forced repayment, independent of the caller, so it is both easy to reach and repeatedly exploitable.

Impact:

1. Collateral accounting drift – Global metrics (e.g., collateralization ratios) are overstated, enabling inaccurate positions to avoid or delay liquidation, thereby increasing insolvency risk.
2. DepositCap blockage – The contract continues to believe the cap is saturated even after MYT shares have been removed, so new deposits are rejected, blocking legitimate users and impairing protocol operations.

Proof of Concept:

function testLiquidationReleasesDepositCapHeadroom() external {
        uint256 amount = 100e18;
        address user = address(0xbeef);
        address liquidator = address(0xface);
        address newDepositor = anotherExternalUser;

        uint256 baselineDeposits = alchemist.getTotalDeposited();
        vm.prank(alOwner);
        alchemist.setDepositCap(baselineDeposits + amount);

        vm.startPrank(user);
        SafeERC20.safeApprove(address(vault), address(alchemist), amount);
        uint256 headroom = alchemist.depositCap() - baselineDeposits;
        alchemist.deposit(headroom, user, 0);
        uint256 tokenId = AlchemistNFTHelper.getFirstTokenId(user, address(alchemistNFT));
        uint256 maxBorrowable = alchemist.getMaxBorrowable(tokenId);
        alchemist.mint(tokenId, maxBorrowable, user);
        vm.stopPrank();

        // Lower the MYT share value sharply so the position falls below the liquidation threshold.
        uint256 initialSupply = IERC20(address(mockStrategyYieldToken)).totalSupply();
        IMockYieldToken(mockStrategyYieldToken).updateMockTokenSupply(initialSupply * 2);

        uint256 totalDepositedBefore = alchemist.getTotalDeposited();
        (, uint256 debt,) = alchemist.getCDP(tokenId);
        uint256 collateralValue = alchemist.totalValue(tokenId);
        uint256 currentRatio = collateralValue * alchemist.FIXED_POINT_SCALAR() / debt;
        assertLt(currentRatio, alchemist.collateralizationLowerBound(), "position undercollateralized");

        vm.startPrank(liquidator);
        (uint256 yieldAmount,,) = alchemist.liquidate(tokenId);// here have call to _forcepay && _doLiquidation
        vm.stopPrank();

        assertGt(yieldAmount, 0, "liquidation executed");
        uint256 totalDepositedAfter = alchemist.getTotalDeposited();
        assertLt(totalDepositedAfter, totalDepositedBefore, "collateral moved out");

        uint256 available = alchemist.depositCap() - totalDepositedAfter;
        assertGt(available, 0, "deposit cap headroom available");

        vm.startPrank(newDepositor);
        SafeERC20.safeApprove(address(vault), address(alchemist), available);
        bool depositReverted;
        uint256 totalDepositedBeforeRetry = alchemist.getTotalDeposited();
        try alchemist.deposit(available, newDepositor, 0) returns (uint256 debtValue) {
            depositReverted = false;
            assertGt(debtValue, 0, "deposit should mint debt value after fix");
        } catch (bytes memory revertData) {
            depositReverted = true;
            bytes4 selector = revertData.length >= 4 ? bytes4(revertData) : bytes4(0);
            assertEq(selector, IllegalState.selector, "unexpected revert reason");
        }
        vm.stopPrank();

        if (depositReverted) {
            assertLt(alchemist.getTotalDeposited(), alchemist.depositCap(), "deposit still blocked pre-fix");
        } else {
            assertEq(alchemist.getTotalDeposited(), totalDepositedBeforeRetry + available, "deposit should increase balance after fix");
            assertLe(alchemist.getTotalDeposited(), alchemist.depositCap(), "deposit cap exceeded after fix");
        }
    }

Recommendation:

In both _forceRepay and _doLiquidation, subtract the exact MYT amount that leaves the contract (principal and any fees) from _mytSharesDeposited, so the internal ledger stays aligned with the actual balance.

function _forceRepay(uint256 accountId, uint256 amount) internal returns (uint256) {
        ......
        ......

        emit ForceRepay(accountId, amount, creditToYield, protocolFeeTotal);

        uint256 protocolFeePaid = 0;
        if (account.collateralBalance > protocolFeeTotal) {
            account.collateralBalance -= protocolFeeTotal;
            // Transfer the protocol fee to the protocol fee receiver
            TokenUtils.safeTransfer(myt, protocolFeeReceiver, protocolFeeTotal);
            protocolFeePaid = protocolFeeTotal;
        }

        uint256 mytOut = protocolFeePaid;
        if (creditToYield > 0) {
            // Transfer the repaid tokens from the account to the transmuter.
            TokenUtils.safeTransfer(myt, address(transmuter), creditToYield);
            mytOut += creditToYield;
        }

        if (mytOut != 0) {
            _mytSharesDeposited -= mytOut;
        }
        return creditToYield;
    }

function _doLiquidation(uint256 accountId, uint256 collateralInUnderlying, uint256 repaidAmountInYield)
        internal
        returns (uint256 amountLiquidated, uint256 feeInYield, uint256 feeInUnderlying)
    {
        ......
        ......
        // update user balance and debt
        account.collateralBalance = account.collateralBalance > amountLiquidated ? account.collateralBalance - amountLiquidated : 0;
        _subDebt(accountId, debtToBurn);

        // send base fee to liquidator if available
        uint256 feePaidInYield = 0;
        if (feeInYield > 0 && account.collateralBalance >= feeInYield) {
            TokenUtils.safeTransfer(myt, msg.sender, feeInYield);
            feePaidInYield = feeInYield;
        }

        // send liquidation amount minus paid fee to transmuter
        uint256 transmuterPayout = amountLiquidated - feePaidInYield;
        if (transmuterPayout > 0) {
            TokenUtils.safeTransfer(myt, transmuter, transmuterPayout);
        }

        uint256 mytOut = transmuterPayout + feePaidInYield;
        if (mytOut != 0) {
            _mytSharesDeposited -= mytOut;
        }

        // Handle outsourced fee from vault
        if (outsourcedFee > 0) 
        .......
        .......
    }

Vechain

Double effective-stake decrement freezes unstake permanently after validator exit

Severity: Critical

Context: Stargate.sol#L266-#L283, #L568

Summary:

Calling requestDelegationExit records a first effective-stake decrement, util validator status changed to EXITED, USER try to call unstake , and enters the EXITED/PENDING branch and applies a second decrement for the same period, causing an underflow (panic 0x11) before any transfers.

Vulnerability Detail:

• First decrement: requestDelegationExit calls _updatePeriodEffectiveStakefor the next period (Stargate.sol #L568).
• Second decrement: unstake in the currentValidatorStatus == EXITED || status == PENDING branch calls _updatePeriodEffectiveStake again Stargate.sol (#L266-#L283), leading to underflow when the checkpointed value is already zero.

Impact:

Once triggered, every unstake attempt reverts; delegations cannot be changed, so the user’s staked VET remains locked in the staking contract. Funds are not stolen but are permanently frozen.

Affected flow: stake + delegate → requestDelegationExit (first decrement) → validator becomes EXITED → any unstake reverts on second decrement.

Proof of Concept:

// PoC: double effective-stake decrement on exit path makes unstake revert (logged flow, no asserts)
    it("poc: double decrement keeps reverting unstake even on repeated attempts", async () => {
        const userAddress = user.address;
        const fmt = (wei: bigint) => ethers.formatEther(wei);

        const balUserStart = await ethers.provider.getBalance(userAddress);
        const balContractStart = await ethers.provider.getBalance(stargateContract);
        console.log(
            `start balances => user: ${fmt(balUserStart)} VET, contract: ${fmt(balContractStart)} VET`
        );

        const levelSpec = await stargateNFTMock.getLevel(LEVEL_ID);
        tx = await stargateContract.connect(user).stake(LEVEL_ID, {
            value: levelSpec.vetAmountRequiredToStake,
        });
        await tx.wait();
        const tokenId = await stargateNFTMock.getCurrentTokenId();
        console.log(`tokenId minted: ${tokenId.toString()}`);

        tx = await stargateContract.connect(user).delegate(tokenId, validator.address);
        await tx.wait();
        console.log("delegated to validator");

        tx = await protocolStakerMock.helper__setValidationCompletedPeriods(validator.address, 2);
        await tx.wait();
        console.log("validator completedPeriods set to 2 (delegation ACTIVE)");

        tx = await stargateContract.connect(user).requestDelegationExit(tokenId);
        await tx.wait();
        console.log("requested exit (first decrement applied)");

        tx = await protocolStakerMock.helper__setValidatorStatus(
            validator.address,
            VALIDATOR_STATUS_EXITED
        );
        await tx.wait();
        console.log("validator forced to EXITED before unstake");

        const balUserPreUnstake = await ethers.provider.getBalance(userAddress);
        const balContractPreUnstake = await ethers.provider.getBalance(stargateContract);
        console.log(
            `before unstake => user: ${fmt(balUserPreUnstake)} VET, contract: ${fmt(balContractPreUnstake)} VET`
        );

        for (let i = 1; i <= 3; i++) {
            try {
                console.log(`unstake attempt ${i}`);
                await stargateContract.connect(user).unstake(tokenId);
                console.log("unexpectedly succeeded");
            } catch (error: any) {
                console.log(`unstake attempt ${i} reverted: ${error.shortMessage || error.message}`);
            }
        }

        const balUserAfter = await ethers.provider.getBalance(userAddress);
        const balContractAfter = await ethers.provider.getBalance(stargateContract);
        console.log(
            `after failed unstakes => user: ${fmt(balUserAfter)} VET, contract: ${fmt(balContractAfter)} VET`
        );
    });

Recommand:

add a status judge before call _updatePeriodEffectiveStake in unstake.

bool shouldDecrement = (delegation.status == DelegationStatus.PENDING) ||
    (currentValidatorStatus == VALIDATOR_STATUS_EXITED && delegation.status == DelegationStatus.NONE); // or some other scenery can do decreament
if (shouldDecrement) {
    _updatePeriodEffectiveStake(..., false);
} else {
    // status == EXITED has been decreased in requestDelegationExit, skip
}