code-423n4 / 2023-09-delegate-findings

2 stars 1 forks source link

`flashloan()` allows both owner and approver to call #256

Closed c4-submissions closed 1 year ago

c4-submissions commented 1 year ago

Lines of code

https://github.com/code-423n4/2023-09-delegate/blob/main/src/DelegateToken.sol#L390 https://github.com/code-423n4/2023-09-delegate/blob/main/src/libraries/DelegateTokenStorageHelpers.sol#L123

Vulnerability details

Impact

Allowing the borrower to borrow more than the current limit

Proof of Concept

As we can see ,flashloan() uses StorageHelpers.revertNotOperator() to check if the msg.sender has permissions to call.

    function flashloan(Structs.FlashInfo calldata info) external payable nonReentrant {
        StorageHelpers.revertNotOperator(accountOperator, info.delegateHolder);//@audit

and the StorageHelpers.revertNotOperator() will check if info.delegateHolder is owner or approver.

    function revertNotOperator(mapping(address account => mapping(address operator => bool enabled)) storage accountOperator, address account) internal view {
        if (msg.sender == account || accountOperator[account][msg.sender]) return;
        revert Errors.NotOperator(msg.sender, account);
    }

Under normal circumstances, if the user deposits the corresponding erc20 token, he can get the delegate (ERC721) token, and can use this token to call flash loan. The loan limit of flash loan depends on how much money the user deposits.

However, let's imagine a scenario:

  1. Alice deposits 100 USDC, assuming she can borrow 80 USDC through flash loan
  2. Alice approved Bob to use her delegate Token, so Bob can also borrow 80 USDC through flash loan
  3. Without approval, user Alice can only borrow 80 USDC. However, after approval to Bob (let us assume that Bob is another account of Alice), then Alice can borrow 80+80=160 USDC. This will It is unfair to upset the balance of this flash loan

Tools Used

manual

Recommended Mitigation Steps

I suggest that we should check that if the owner approves it to another user, the owner is not allowed to use flash loan

Assessed type

Access Control

GalloDaSballo commented 1 year ago

Should have had a POC

c4-sponsor commented 1 year ago

0xfoobar (sponsor) disputed

0xfoobar commented 1 year ago

This is desired behavior

GalloDaSballo commented 1 year ago

After re-checking the finding, I agree with the Sponsor

c4-judge commented 1 year ago

GalloDaSballo marked the issue as unsatisfactory: Invalid