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

8 stars 7 forks source link

Unnecessary payable in `executeFlashloan()` #296

Open code423n4 opened 1 year ago

code423n4 commented 1 year ago

Lines of code

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

Vulnerability details

Impact

This could lead to users losing the eth sent with this transaction, which would be better served by reverting instead of accepting a non-zero value of msg.value

Proof of Concept

There are 2 instances of payble function in the PeUSDMainnetStableVision.sol contract, after an extensive discussion with the team we came to an understanding that unlike the convertToPeUSDAndCrossChain() function that needs to call OFT's sendFrom, which requires passing gas to lzEndpoint the payable inside executeFlashloan() can be removed.

Take a look at both functions: convertToPeUSDAndCrossChain()

    /**
     * @dev Allows users to deposit eUSD and mint PeUSD tokens, which can be directly bridged to other networks.
     * @param eusdAmount The amount of eUSD to deposit and mint PeUSD tokens.
     * @param dstChainId The chain ID of the target network.
     * @param toAddress The receiving address after cross-chain transfer.
     * @param callParams Additional parameters.
     */
    function convertToPeUSDAndCrossChain(
        uint256 eusdAmount,
        uint16 dstChainId,
        bytes32 toAddress,
        LzCallParams calldata callParams
    ) external payable {
        convertToPeUSD(_msgSender(), eusdAmount);
        sendFrom(_msgSender(), dstChainId, toAddress, eusdAmount, callParams);
    }

executeFlashloan()

    /**
     * @dev Allows users to lend out any amount of eUSD for flash loan calls.
     * @param receiver The address of the contract that will receive the borrowed eUSD.
     * @param eusdAmount The amount of eUSD to lend out.
     * @param data The data to be passed to the receiver contract for execution.
     */
    function executeFlashloan(FlashBorrower receiver, uint256 eusdAmount, bytes calldata data) public payable {
        uint256 shareAmount = EUSD.getSharesByMintedEUSD(eusdAmount);
        EUSD.transferShares(address(receiver), shareAmount);
        receiver.onFlashLoan(shareAmount, data);
        bool success = EUSD.transferFrom(address(receiver), address(this), EUSD.getMintedEUSDByShares(shareAmount));
        require(success, "TF");

        uint256 burnShare = getFee(shareAmount);
        EUSD.burnShares(address(receiver), burnShare);
        emit Flashloaned(receiver, eusdAmount, burnShare);
    }

Tool used

Manual Audit

Recommended Mitigation Steps

Simple fix, remove the payable

Assessed type

Payable

c4-pre-sort commented 1 year ago

JeffCX marked the issue as duplicate of #112

c4-judge commented 1 year ago

0xean changed the severity to QA (Quality Assurance)

c4-judge commented 1 year ago

0xean marked the issue as grade-b