code-423n4 / 2021-04-vader-findings

1 stars 0 forks source link

ERC20 race condition for allowances #35

Open code423n4 opened 3 years ago

code423n4 commented 3 years ago

Handle

toastedsteaksandwich

Vulnerability details

Impact

Due to the implementation of the approve() function in Vader.sol, Vether.sol and mainnet Vether4 at 0x4Ba6dDd7b89ed838FEd25d208D4f644106E34279, it's possible for a user to over spend their allowance in certain situations.

Proof of Concept

The steps to the attack are as follows:

1) Alice approves an allowance of 5000 VETH to Bob. 2) Alice attempts to lower the allowance to 2500 VETH. 3) Bob notices the transaction in the mempool and front-runs it by using up the full allowance with a transferFrom call. 4) Alice's lowered allowance is confirmed and Bob now has an allowance of 2500 VETH, which can be spent further for a total of 7500 VETH.

Overall, Bob was supposed to be approved for at most 5000 VETH but got 7500 VETH. The POC code can be found here: https://gist.github.com/toastedsteaksandwich/db32472ae5c19c2eb188f07abddd02fa

Note that in the POC, Bob receives 7492.5 VETH instead of 7500 VETH due to transfer fees.

Tools Used

Hardhat with mainnet forks, pinned to block 12227519.

Recommended Mitigation Steps

Instead of having a direct setter for allowances, decreaseAllowance and increaseAllowance functions should be exposed which decreases and increases allowances for a recipient respectively. In this way, if the decreaseAllowance call is front-run, the call should revert as there is insufficient allowance to be decreased. This leaves Bob with at most 5000 VETH, the original allowance.

strictly-scarce commented 3 years ago

This theoretical attack has been known about for a while but never actually observed meaningfully. Addressing it costs extra gas.