0x52 - Malicious user can grief withdrawing users via VUSD reentrancy

[sherlock-admin]

0x52

medium

Malicious user can grief withdrawing users via VUSD reentrancy

Summary

VUSD#processWithdraw makes a call to withdrawal.usr to send the withdrawn gas token. processWithdrawals is the only nonreentrant function allowing a user to create a smart contract that uses it's receive function to deposit then immediately withdraw to indefinitely lengthen the withdrawal queue and waste large amounts of caller gas.

Vulnerability Detail

VUSD.sol#L69-L77

    while (i < withdrawals.length && (i - start) < maxWithdrawalProcesses) {
        Withdrawal memory withdrawal = withdrawals[i];
        if (reserve < withdrawal.amount) {
            break;
        }

        (bool success, bytes memory data) = withdrawal.usr.call{value: withdrawal.amount}("");
        if (success) {
            reserve -= withdrawal.amount;

To send the withdrawn gas token to the user VUSD#processWithdrawals utilizes a call with no data. When received by a contract this will trigger it's receive function. This can be abused to continually grief users who withdraw with no recurring cost to the attacker. To exploit this the attacker would withdraw VUSD to a malicious contract. This contract would deposit the received gas token then immediately withdraw it. This would lengthen the queue. Since the queue is first-in first-out a user would be forced to process all the malicious withdrawals before being able to process their own. While processing them they would inevitably reset the grief for the next user.

NOTE: I am submitting this as a separate issue apart from my other two similar issues. I believe it should be a separate issue because even though the outcome is similar the root cause is entirely different. Those are directly related to the incorrect call parameters while the root cause of this issue is that both mintWithReserve and withdraw/withdrawTo lack the reentrant modifier allowing this malicious reentrancy.

Impact

Malicious user can maliciously reenter VUSD to grief users via unnecessary gas wastage

Code Snippet

VUSD.sol#L45-L48

VUSD.sol#L50-L52

VUSD.sol#L58-L60

Tool used

Manual Review

Recommendation

Add the nonreentrant modifer to mintWithReserve withdraw and withdrawTo

[0xshinobii]

Will add the nonreentrant modifer to mintWithReserve withdraw and withdrawTo

[ctf-sec]

To exploit this the attacker would withdraw VUSD to a malicious contract. This contract would deposit the received gas token then immediately withdraw it.

I put this issue and #160 together because these two issue highlight different ways of wasting gas, but they also focus on how to waste gas in external call.

Recommend checking #160 as well.

and I leave the #158 as a separate issue because the root cause is the returned call data is emitted in the contract code itself

[0xshinobii]

yes noted. #160 has slightly different cause but same effect. So the solution for all these related issues is

• don't save data in variable 0x52 - Malicious user can permanently break VUSD#processWithdrawals by returning huge amounts of data #158
• cap the gas limit of .call 0x52 - Malicious user can permanently break VUSD#processWithdrawals by wasting all transaction gas #160
• this

[IAm0x52]

Escalate

As the sponsor has pointed out, this is a different issue from the dupes. While the outcome of wasting gas is similar, the root cause is completely different. The root cause for this is reentrancy across functions, while the root cause of issues marked as dupes is that there is no gas limit. I suggest that this issue be separated and the dupes groped together as separate issues.

Edit:
Missed #195. That and this should be considered a separate issue

[sherlock-admin2]

Escalate

As the sponsor has pointed out, this is a different issue from the dupes. While the outcome of wasting gas is similar, the root cause is completely different. The root cause for this is reentrancy across functions, while the root cause of issues marked as dupes is that there is no gas limit. I suggest that this issue be separated and the dupes groped together as separate issues.

Edit:
Missed #195. That and this should be considered a separate issue

You've created a valid escalation!

To remove the escalation from consideration: Delete your comment.

You may delete or edit your escalation comment anytime before the 48-hour escalation window closes. After that, the escalation becomes final.

[ctf-sec]

See escalation #116

There are a lot of ways to consume all gas in external call (reentrancy to expand the queue size, gas token, for loop, swap, etc....), cannot count each of them as duplicates

I think grouping all these issue about wasting gas in external call to one issue make sense, root cause is gas limit not capped.

[0xArcturus]

Agreed with escalation,
as also mentioned in #195, the root cause of this is the lack of reentrancy guard and the withdrawals.length changing during execution. This attack can bloat the queue by reentering with a single withdraw call, while the dupes are focused on consuming gas in a single call.

[hrishibhat]

Result:
Medium
Has duplicates
Agree with the escalation that this should be a separate issue along with #195

[sherlock-admin2]

Escalations have been resolved successfully!

Escalation status:

• IAm0x52: accepted

[0xshinobii]

Fixed in this PR. The description is in the PR.

[IAm0x52]

Fix looks good. mintWithReserve, withdraw, and withdrawTo now use the nonreentrant modifier to prevent this exploit