code-423n4 / 2022-01-trader-joe-findings

2 stars 0 forks source link

Owner can DoS `withdrawAVAX()` function #276

Closed code423n4 closed 2 years ago

code423n4 commented 2 years ago

Handle

Czar102

Vulnerability details

Impact

The owner of can DoS the withdrawAVAX() function by setting penaltyCollector in RocketJoeFactory to a contract that triggers an infinite loop, or uses gas to perform own operations (can store it to get refunds like in gas token). This way, an owner can use withdrawing wallets to pay for their transactions or DoS the withdrawal function.

Proof of Concept

        if (feeAmount > 0) {
            _safeTransferAVAX(rocketJoeFactory.penaltyCollector(), feeAmount);
        }

Tools Used

Manual analysis

Recommended Mitigation Steps

Add gas limit on the call to penaltyCollector.

CloudEllie commented 2 years ago

Adding further context from warden Czar102, sent via DM:

Also, a crucial modification is not to revert on call failure. Whether it is because of not enough gas in the call context or a deliberate revertion.

cryptofish7 commented 2 years ago

It's assumed that the owner is well-behaved

dmvt commented 2 years ago

I can't come up with a reason that the owner of a launch would DOS their own launch. If they were a bad actor it would be much more lucrative to see the launch through and then rug it. Invalid.