PancakeSwap Logic Error Bug Fix Postmortem

3 years ago 177
BOOK THIS SPACE FOR AD
ARTICLE AD

Immunefi

Summary:

Whitehat Juno submitted a critical vulnerability in PancakeSwap’s lottery contract on April 27. The vulnerability consisted of a logic error. Due to insufficient validation, a malicious user could have claimed the same winning ticket at least 255 times in a single transaction, meaning the reward size was 255 times too great. A total of $700,000 in funds could have been lost, had the vulnerability been exploited. There is no evidence the vulnerability was ever exploited. Prior to the reporting of this vulnerability, PancakeSwap had already shut down its lottery contract by disallowing new lottery drawings.

After disclosure, PancakeSwap fixed the vulnerability, and Immunefi began the process of disclosure assistance, contacting every project on Binance Smart Chain that had forked PancakeSwap’s lottery contract to inform them of the vulnerability as well. ApeSwap, PantherSwap, and Knights DeFi were vulnerable. All three decided to pay a bounty to the whitehat as well. PancakeSwap paid $70,000 in CAKE tokens. ApeSwap paid $2,700. PantherSwap paid $1,324. Knights DeFi paid 7,000 KNIGHT tokens. All affected projects have fixed the vulnerability.

Vulnerability Analysis:

The logic error vulnerability was present in Lottery.sol:

function multiClaim(uint256[] memory _tickets) external { uint256 totalReward = 0; for (uint i = 0; i < _tickets.length; i++) { require (msg.sender == lotteryNFT.ownerOf(_tickets[i]), “not from owner”); require (!lotteryNFT.getClaimStatus(_tickets[i]), “claimed”); uint256 reward = getRewardView(_tickets[i]); if(reward>0) { totalReward = reward.add(totalReward); }}lotteryNFT.multiClaimReward(_tickets); if(totalReward>0) { cake.safeTransfer(address(msg.sender), totalReward); } emit MultiClaim(msg.sender, totalReward);}

An analysis of this code block reveals that if a ticket is submitted to multiClaim multiple times (that is, the _tickets[] aren’t all unique), the duplicate ticket will be able to be redeemed multiple times. This happens because Lottery.sol checks that the ticket hasn’t been claimed, and it checks all the tickets before marking any of them as claimed. Additionally LotteryNFT.sol multiClaimReward doesn’t require that the ticket being claimed hasn’t already been marked as claimed.

None of the checks account for the possibility that a malicious user would pass the same ticket multiple times. The result is that the total reward is a function of the reward itself multiplied by the number of times the ticket is claimed.

The step by step exploitation of the vulnerability is as follows:

Step 1: Buy a lottery ticket

Step 2: Win the lottery. The lottery works by allowing you to pick a number between 1 and 14, and you pick 4 numbers. There are prizes available for matching at least two of the numbers together. If you match four numbers together, you win the grand prize. However, the exploit does not depend on winning the grand prize. The exploit depends on winning at least some amount of money, which is not difficult.

Step 3: Submit the malicious transaction, which calls multiClaim(), and pass the winning ticket 255 times, where each ticket is the ID of the NFT lottery ticket. There’s no validation that a user is claiming the same ticket, so that user can effectively claim the same ticket an unlimited number of tickets — with the only limit being the blocksize of the transaction.

Vulnerability Fix:

Prior to the reporting of this vulnerability, PancakeSwap had already shut down its lottery contract by disallowing new lottery drawings. However, even though no additional drawings were held, the contract still had approximately $700,000 in funds remaining because users still had outstanding tickets to claim. The simultaneous existence of the bug and the funds locked up would have made the contract worth exploiting by a malicious user. After Juno reported the vulnerability through Immunefi, PancakeSwap withdrew all funds out of the contract.

PantherSwap, however, kept the lottery contract active and applied a fix as suggested by Immunefi CTO Duncan Townsend. The fix was to rewrite the multiClaim() function as follows:

function multiClaim(uint256[] memory _tickets) external { uint256 totalReward = 0; for (uint i = 0; i < _tickets.length; i++) { require (msg.sender == lotteryNFT.ownerOf(_tickets[i]), “not from owner”); require (!lotteryNFT.getClaimStatus(_tickets[i]), “claimed”); lotteryNFT.claimReward(_tickets[i]); uint256 reward = getRewardView(_tickets[i]); if(reward>0) { totalReward = reward.add(totalReward); } } if(totalReward>0) { panther.safeTransfer(address(msg.sender), totalReward); } emit MultiClaim(msg.sender, totalReward);}

ApeSwap shut down its lottery contract.

Knights DeFi took down the contract and migrated to a new one.

Acknowledgments:

We’d like to thank Pancakeswap, ApeSwap, PantherSwap, and Knights DeFi for their rapid and effective response to the bug report and subsequent disclosure assistance efforts. PancakeSwap paid out a bounty of $70,000 to the whitehat. ApeSwap, PantherSwap, and Knights DeFi also paid out bounties to the whitehat and would like to thank Immunefi for helping facilitate the disclosure process. To report additional vulnerabilities, please see PancakeSwap’s bug bounty program with Immunefi. If you’re interested in protecting your project with a bug bounty like PancakeSwap, visit the Immunefi services page and fill out the form.

Read Entire Article