search

code-423n4 / 2023-09-delegate-findings

2 stars 1 forks source link

The DelegateToken contract is vulnerable to draining assets via repeated flashloans of the same assets #270

Closed c4-submissions closed 1 year ago

c4-submissions commented 1 year ago

Lines of code

https://github.com/code-423n4/2023-09-delegate/blob/a6dbac8068760ee4fc5bababb57e3fe79e5eeb2e/src/DelegateToken.sol#L393 https://github.com/code-423n4/2023-09-delegate/blob/a6dbac8068760ee4fc5bababb57e3fe79e5eeb2e/src/DelegateToken.sol#L399 https://github.com/code-423n4/2023-09-delegate/blob/a6dbac8068760ee4fc5bababb57e3fe79e5eeb2e/src/DelegateToken.sol#L406 https://github.com/code-423n4/2023-09-delegate/blob/a6dbac8068760ee4fc5bababb57e3fe79e5eeb2e/src/DelegateToken.sol#L396 https://github.com/code-423n4/2023-09-delegate/blob/a6dbac8068760ee4fc5bababb57e3fe79e5eeb2e/src/DelegateToken.sol#L402 https://github.com/code-423n4/2023-09-delegate/blob/a6dbac8068760ee4fc5bababb57e3fe79e5eeb2e/src/DelegateToken.sol#L408

Vulnerability details

Impact

This drains the asset out of the contract.

Proof of Concept

The DelegateToken contract is vulnerable to draining assets via repeated flashloans of the same assets. The key parts of the code that enable this are:

  1. No checks that flashloaned assets are returned: https://github.com/code-423n4/2023-09-delegate/blob/a6dbac8068760ee4fc5bababb57e3fe79e5eeb2e/src/DelegateToken.sol#L393 https://github.com/code-423n4/2023-09-delegate/blob/a6dbac8068760ee4fc5bababb57e3fe79e5eeb2e/src/DelegateToken.sol#L399 https://github.com/code-423n4/2023-09-delegate/blob/a6dbac8068760ee4fc5bababb57e3fe79e5eeb2e/src/DelegateToken.sol#L406 The assets are transferred out via transferFrom and safeTransferFrom, but there is no verification that the assets are returned.

  2. Pulling assets back in after flashloan without checking balances: https://github.com/code-423n4/2023-09-delegate/blob/a6dbac8068760ee4fc5bababb57e3fe79e5eeb2e/src/DelegateToken.sol#L396 https://github.com/code-423n4/2023-09-delegate/blob/a6dbac8068760ee4fc5bababb57e3fe79e5eeb2e/src/DelegateToken.sol#L402 https://github.com/code-423n4/2023-09-delegate/blob/a6dbac8068760ee4fc5bababb57e3fe79e5eeb2e/src/DelegateToken.sol#L408

The assets are pulled back after the flashloan via pull functions, but balances are not checked before pulling back in. So an attacker could:

  1. Flashloan an asset
  2. Not return the flashloaned asset
  3. Call the flashloan function again for the same asset
  4. The contract pulls the asset back in, even though the balance is now 0

Tools Used

Manual

Recommended Mitigation Steps

Balances of the assets should be checked before and after flashloans to ensure the proper amounts are maintained

Assessed type

Other

c4-judge commented 1 year ago

GalloDaSballo marked the issue as unsatisfactory: Invalid