A malicious contract could steal assets via a flash loan

[c4-submissions]

Lines of code

https://github.com/code-423n4/2023-09-delegate/blob/a6dbac8068760ee4fc5bababb57e3fe79e5eeb2e/src/DelegateToken.sol#L389-L408 https://github.com/code-423n4/2023-09-delegate/blob/a6dbac8068760ee4fc5bababb57e3fe79e5eeb2e/src/DelegateToken.sol#L391 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#L397 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#L403 https://github.com/code-423n4/2023-09-delegate/blob/a6dbac8068760ee4fc5bababb57e3fe79e5eeb2e/src/DelegateToken.sol#L406

Vulnerability details

Impact

A malicious contract could fail to return the assets, essentially stealing the

Proof of Concept

The key vulnerability is in the flashloan function. It transfers the assets to the receiver contract specified in info.receiver without any checks. Then it calls Helpers.revertOnCallingInvalidFlashloan(info) to check if the receiver callback was valid. A malicious receiver contract could:

1. Receive the flashloaned assets
2. NOT call back to the flashloan function
3. End up keeping the assets since they were transferred without any checks This is possible because: • The assets are transferred via transferFrom or safeTransfer to the receiver WITHOUT checking if the callback will be made • Helpers.revertOnCallingInvalidFlashloan(info) reverts if the callback was NOT made back to flashloan. But since execution has already passed this point, reverting here does not undo the asset transfers. So in summary:
4. Assets are flashloaned to receiver
5. Receiver does NOT callback
6. Execution continues and reaches Helpers.revertOnCallingInvalidFlashloan
7. This reverts BUT does not undo the asset transfers that already happened
8. Receiver keeps the assets

Tools Used

Manual

Recommended Mitigation Steps

1. Transfer assets via a low-level call and check the return value - this allows undoing the transfer if callback fails
2. Alternatively, hold the assets in escrow during the flash loan, only releasing them if the callback succeeds.

Assessed type

Other

[c4-judge]

GalloDaSballo marked the issue as unsatisfactory: Invalid