Flashloan function of the PeUSD contract is dangerous

[code423n4]

Lines of code

https://github.com/code-423n4/2023-06-lybra/blob/7b73ef2fbb542b569e182d9abf79be643ca883ee/contracts/lybra/token/PeUSDMainnetStableVision.sol#L129-L139

Vulnerability details

Impact

The PeUSDMainnetStableVision contract has a function called executeFlashloan, which is used to temporarily request some amount of the EUSD accumulated in the contract, which should be returned plus a fee in the same transaction.

Normally these functions follow the pattern of taking a snapshot of the contract's balance before and after the external call to the receiver of the flashloan. But executeFlashloan uses a pull pattern via:

EUSD.transferShares(address(receiver), shareAmount);
receiver.onFlashLoan(shareAmount, data);
bool success = EUSD.transferFrom(address(receiver), address(this), EUSD.getMintedEUSDByShares(shareAmount));
require(success, "TF");

It takes the funds from the receiver via transferFrom. The problem is that the PeUSD contract has privileges over the EUSD contract and can call transferFrom over an arbitrary account's funds without approval.

Proof of Concept

If a contract has a fallback function that does not revert, which is not rare to see, and this contract holds EUSD. Then any user can request flashloans on behalf of this contract draining its funds due to the flashloan fee.

Justification

Although the issue has some external requirements, like a smart contract with a fallback function that does not revert or a smart contract with a weak implementation of the onFlashLoan function, I landed a medium on this finding for the following reasons:

• The PeUSD contract can execute the transferFrom function of the EUSD contract without the approval of the owner of the tokens, making the executeFlashLoan function risky to smart contracts that hold EUSD tokens, damaging the composability and reachability of EUSD.

• Mitigation is easy (see mitigation section).

Tools Used

Manual Review

Recommended Mitigation Steps

Option 1:

Do not allow the PeUSD contract to have privileges on the transferFrom function in the EUSD contract.

Option 2:

Add the following line to the executeFlashloan function:

require(EUSD.allowances(receiver, address(this)) >= EUSD.getMintedEUSDByShares(shareAmount));

Option 3:

Use the snapshots pattern

Assessed type

Rug-Pull

[c4-pre-sort]

JeffCX marked the issue as duplicate of #280

[c4-judge]

0xean marked the issue as satisfactory

[c4-judge]

0xean changed the severity to 3 (High Risk)