code-423n4 / 2023-06-lybra-findings

8 stars 7 forks source link

Any native ether sent to `executeFlashLoan` is irrevocably lost #112

Open code423n4 opened 1 year ago

code423n4 commented 1 year ago

Lines of code

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

Vulnerability details

Impact

Users sending Ether along with the executeFlashLoan function call unintentionally or as part of the intended action lose their sent Ether, as it gets locked in the contract permanently without a way to recover it.

Proof of Concept

In the PeUSDMainnet contract, the executeFlashLoan function is defined as:

 function executeFlashloan(FlashBorrower receiver, uint256 eusdAmount, bytes calldata data) public payable {

This function is marked as payable, which means it is able to receive Ether (ETH). However, within the function body, there is no logic that accounts for msg.value being sent. Additionally, the function does not refund any Ether sent as msg.value, which might be redundant or sent by mistake. As the contract does not have a mechanism to send back the received Ether, any Ether sent to this function is irrecoverably locked within the contract.

Tools Used

Recommended Mitigation Steps

Remove the payable modifier if the function is not intended to receive Ether. This prevents accidental sending of Ether to the function.

If the function is meant to accept Ether, implement logic within the function to handle the received Ether appropriately.

Assessed type

Other

c4-pre-sort commented 1 year ago

JeffCX marked the issue as primary issue

JeffCX commented 1 year ago

QA

c4-pre-sort commented 1 year ago

JeffCX marked the issue as low quality report

c4-judge commented 1 year ago

0xean marked the issue as unsatisfactory: Overinflated severity

c4-judge commented 1 year ago

0xean changed the severity to QA (Quality Assurance)