Mt Pelerin Double Transaction Bugfix Review

Summary

On September 21, 2022, an anonymous whitehat reported a critical severity bug in Mt Pelerin’s bridge-protocol-v2 via Immunefi, which could have allowed an attacker to drain the contract of funds.

However, thanks to the efforts of the whitehat, no user funds were lost. Mt Pelerin quickly acknowledged the vulnerability, fixed the bug, and paid out a $10,000 bounty.

In short, the whitehat discovered a vulnerability in the cancelOnHoldTransfers function. This function normally allows anyone to cancel their on-hold transfers, which are non-compliant token transfers detected by the Bridge Protocol’s processor and placed on hold until compliance staff can review the transaction and approve or reject it.

The contract was vulnerable in a way that could result in a loss of funds because the function failed to verify if the transaction was repeated or marked as canceled already.

This means that a malicious hacker could have provided an array with the same transaction several times and drain the contract because the contract fails to determine if the user-provided transaction has been already canceled and instead of reverting it will send additional funds to the user.

Vulnerability Analysis

Mt Pelerin’s ComplianceRegistry.sol smart contract is responsible for the storage of all identity information linked to an address and the storage of the history of transfers linked to an address.

The Compliance Registry is responsible for the storage of all identity information linked to an address or the storage of the history of transfers linked to an address. The compliance registry is managed by trusted intermediaries, the KYC/AML providers. Each trusted intermediary has its own space within the registry to update its own address-related information. Based on the token trusted intermediaries, the Compliance Registry will return the compliance information that has been updated by one of the token trusted intermediaries.

The contract has a function named cancelOnHoldTransfer which could be called by any user to cancel transfers for a specific trusted intermediary to get their tokens back. The function required:

1) the user to provide the address of the trusted intermediary,

2) an array of transactions to be canceled, and

3) skipMinBoundaryUpdate, which determines whether or not to skip the minBoundary which is an index used to limit iterations to avoid an out of gas exceptions update.

Snippet 1: the vulnerable code

The cancelOnHoldTransactions() function has a for loop that loops over the array of transactions that the user has requested to be canceled, verifies that the transfer.from and msg.sender are the same, changes the transaction’s status to cancel, transfers the token to the recipient, and finally emits a TransferCancelled event. To summarize, the workflow looks like the following:

1. The attacker calls the cancelOnHoldTransactions() function with an array of identical transactions to be canceled.
2. The contract then loops through the user-supplied transaction array, ensuring that the sender of that transaction matches msg.sender.
3. Once it confirms that the transaction was initiated by the same user, it cancels the transfer but does not check to see if the transaction has previously been canceled and instead simply transfers the token and emits the TransferCancelled event.
4. The function then repeats the process with the next array element which is again the same transaction and the function does not have a check to verify if it’s already been canceled but instead allows an attacker to steal funds from the contract.

When we examine the function closely, we can see that it appears to be missing a security check to see if the transaction has already been canceled. So, if given an array of the same repeated transfers as parameters, the function will proceed through the loop without checking to see if it has already been canceled until all funds have been drained.

Vulnerability Fix

GitHub Commit

Mt Pelerin fixed the vulnerability by adding a check which verifies that the status of the transfer is on hold and not already canceled.

Acknowledgements

We would like to thank the anonymous whitehat for doing an amazing job and responsibly disclosing such an important bug. Big props also to the Mt Pelerin team who responded quickly to the report and patched it.

Thanks also to Rudra of Immunefi for writing this bugfix review.

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.

And if you’re feeling good about your skillset and want to see if you will find bugs in the code, check out the bug bounty program from Mt Pelerin.