WATCHPUG - `exchangeFee` can be escaped

[sherlock-admin]

WATCHPUG

medium

exchangeFee can be escaped

Summary

Comparing the before and after balance of the swap call for the swapped amount can be exploited to escape the exchangeFee by wrapping the actual swap inside a fake swap.

Vulnerability Detail

The attacker can reenter with another CardTopupPermit() -> _processTopup() -> IExchangeProxy#executeSwapDirect() at L174 to claw back the fee:

1. Swap minAmount with 1inch, inside the 1inch swap at ExchangeProxy.sol#L174, reenter and HardenedTopupProxy.sol#CardTopupPermit();
2. The inner swap is the actual amount: $1M, which should pay for $1000 exchangeFee;
3. After the inner swap, amountReceived includes the $1000 exchangeFee, which will be sent back to the user.

As a result, the user successfully escaped most of the exchangeFee.

Impact

User can escape the exchangeFee.

Code Snippet

https://github.com/sherlock-audit/2022-10-mover/blob/main/cardtopup_contract/contracts/HardenedTopupProxy.sol#L336-L343

https://github.com/sherlock-audit/2022-10-mover/blob/main/cardtopup_contract/contracts/ExchangeProxy.sol#L160-L185

Tool used

Manual Review

Recommendation

Consider adding nonReentrant() modifier to all the 3 non-view methods in the HardenedTopupProxy:

• CardTopupPermit();
• CardTopupTrusted();
• CardTopupMPTProof().

[McMannaman]

This is a valid point. Would be fixed by adding nonReentrant modifiers.

[jack-the-pug]

Fix confirmed

[McMannaman]

The fixes are in https://github.com/viaMover/2022-10-mover/pull/2