Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

borrowing unwanted flashloan on behalf of receiver #503

Closed
code423n4 opened this issue Jul 2, 2023 · 3 comments
Closed

borrowing unwanted flashloan on behalf of receiver #503

code423n4 opened this issue Jul 2, 2023 · 3 comments
Labels
3 (High Risk) Assets can be stolen/lost/compromised directly bug Something isn't working duplicate-769 edited-by-warden satisfactory satisfies C4 submission criteria; eligible for awards upgraded by judge Original issue severity upgraded from QA/Gas by judge

Comments

@code423n4
Copy link
Contributor

code423n4 commented Jul 2, 2023

Lines of code

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

Vulnerability details

Impact

There is no control on borrowing flashloan, so anyone can borrow an unwanted flashloan on behalf of a receiver, and the receiver must burn share to pay the fee (in case it has nonzero eUSD balance).

Proof of Concept

Suppose there is an innocent contract (called FlashloanBorrower.sol) which is deployed to borrow flashloan from LybraFinance protocol. This contract must have the interface FlashBorrower implemented to be used during the flashloan callback:

interface FlashBorrower {
    /// @notice Flash loan callback
    /// @param amount The amount of tokens received
    /// @param data Forwarded data from the flash loan request
    /// @dev Called after receiving the requested flash loan, should return tokens + any fees before the end of the transaction
    function onFlashLoan(uint256 amount, bytes calldata data) external;
}

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

During executing flashloan, the requested amount is transferred to the receiver:
https://github.com/code-423n4/2023-06-lybra/blob/7b73ef2fbb542b569e182d9abf79be643ca883ee/contracts/lybra/token/PeUSDMainnetStableVision.sol#L131
Then, the callback function is called on the receiver address:
https://github.com/code-423n4/2023-06-lybra/blob/7b73ef2fbb542b569e182d9abf79be643ca883ee/contracts/lybra/token/PeUSDMainnetStableVision.sol#L132
Then, the same lent out fund will be transferred from the receiver to the protocol:
https://github.com/code-423n4/2023-06-lybra/blob/7b73ef2fbb542b569e182d9abf79be643ca883ee/contracts/lybra/token/PeUSDMainnetStableVision.sol#L133
Finally, the fee of the flashloan is burned from the receiver.
https://github.com/code-423n4/2023-06-lybra/blob/7b73ef2fbb542b569e182d9abf79be643ca883ee/contracts/lybra/token/PeUSDMainnetStableVision.sol#L137

The problem is that anyone can call this function to borrow flashloan on behalf of FlashloanBorrower.sol, and finally the fee will be deducted from FlashloanBorrower.sol.

Let's say Bob (a malicious user) calls executeFlashloan(FlashBorrower(address(FlashloanBorrower)), 1_000_000e18, ""). In this case, 1_000_000e18 will be transferred to FlashloanBorrower contract, then the callback is called, then it is transferred from FlashloanBorrower to the protocol, and finally the fee is deducted from FlashloanBorrower (assuming FlashloanBorrower has enough eUSD to pay the fee). As a result, FlashloanBorrower loses the fee of the unwanted flashloan.

Tools Used

Recommended Mitigation Steps

There are two recommendations as other protocol's flashloan is working:

  • First: Taking the fee from the msg.sender, not the receiver. In other words, burning the shares of msg.sender as fee.
  • Second: Instead of directly calling EUSD.transferFrom(...), it is better to track the balanceBefore and balanceAfter, so that balanceAfter - balanceBefore >= getFee(shareAmount).

Assessed type

Context

@code423n4 code423n4 added 2 (Med Risk) Assets not at direct risk, but function/availability of the protocol could be impacted or leak value bug Something isn't working labels Jul 2, 2023
code423n4 added a commit that referenced this issue Jul 2, 2023
@c4-pre-sort
Copy link

JeffCX marked the issue as duplicate of #280

@c4-judge
Copy link
Contributor

0xean marked the issue as satisfactory

@c4-judge c4-judge added satisfactory satisfies C4 submission criteria; eligible for awards duplicate-769 and removed duplicate-280 labels Jul 28, 2023
@c4-judge
Copy link
Contributor

0xean changed the severity to 3 (High Risk)

@c4-judge c4-judge added 3 (High Risk) Assets can be stolen/lost/compromised directly upgraded by judge Original issue severity upgraded from QA/Gas by judge and removed 2 (Med Risk) Assets not at direct risk, but function/availability of the protocol could be impacted or leak value labels Jul 28, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3 (High Risk) Assets can be stolen/lost/compromised directly bug Something isn't working duplicate-769 edited-by-warden satisfactory satisfies C4 submission criteria; eligible for awards upgraded by judge Original issue severity upgraded from QA/Gas by judge
Projects
None yet
Development

No branches or pull requests

3 participants