Flashloan SafeTransferFrom Allows Approval Frontrunning Attack

[c4-bot-9]

Lines of code

https://github.com/code-423n4/2024-02-spectra/blob/383202d0b84985122fe1ba53cfbbb68f18ba3986/src/tokens/PrincipalToken.sol#L628

Vulnerability details

Impact

Flashloaner contracts that approve tokens to the PrincipalToken can have their approved tokens drained by an external attacker.

Proof of Concept

The difference in the flashloan function in PrincipalToken.sol from standard flashloan implementations is that it calls safeTransferFrom from the receiver rather than checking the balanceOf the contract afterwards which allows attackers to drain tokens from the receiver.

To execute a flashloan, a contract must first approve tokens to the PrincipalToken contract.

However, after these approvals are made an attacker can make calls to the flashloaner by by calling flashloan with the victim as target.

Without doing any special function calls, the user could simply drain the fees as the transferFrom requires fees.

Note that the extra fee assets which are deposited into the vault boosts the value of Principal Token shares (and therefore YT). An attacker could capture this as a profit by depositing a large amount of PT before doing the flashloan-draining attack to capture the victim's loss as a profit.

Here is the attack sequence

1. deposit large amount into PrincipalToken.sol
2. call flashloan and enter the victim contract as the receiver and call any external non-reverting function by entering the corresponding calldata. The PrincipalToken contract will send the flashloan amount and then transferFrom the victim the amount+fee. So the victim has overall lost the fee amount to the PrincipalToken contract
3. When attacker redeems the YT/PT/shares, they get more than they deposited due to the extra assets in the PrincipalToken contract.

The fees actually go to the vault share owners.

Tools Used

Manual Review

Recommended Mitigation Steps

Check the balanceOf the contract to ensure it is greater or equal to the original balance + fees.

Assessed type

Token-Transfer

[c4-pre-sort]

gzeon-c4 marked the issue as primary issue

[c4-pre-sort]

gzeon-c4 marked the issue as sufficient quality report

[gzeon-c4]

I don't think this is an issue, the flashloan contract should set approval in the receiver call only

note to judge: marking other potential flashloan receiver implementation error as dupes

[c4-sponsor]

yanisepfl (sponsor) disputed

[c4-sponsor]

yanisepfl marked the issue as disagree with severity

[yanisepfl]

When attacker redeems the YT/PT/shares, they get more than they deposited due to the extra assets in the PrincipalToken contract.

The PTs worth in asset (or ptRate) does not depend on the assets/IBTs amount present in our PT vaults. Therefore, most of what has been stated is wrong.

That being said, the above issue contains a small part of correct information, that is better explained here: https://github.com/code-423n4/2024-02-spectra-findings/issues/113.

Hence we agree there is an issue but strongly disagree with the severity. We consider the real issue as a medium as it is a grief and the fees are possessed by a trusted admin.

[c4-judge]

JustDravee marked the issue as unsatisfactory: Invalid