Blog Post
High Risk Bug Disclosure: Across Bridge Double-Spend
October 12, 2022
## Introduction
On 24 August 2022, [Jason Matthyser](https://twitter.com/pleasew8t), a security researcher at iosiro, identified a high-risk vulnerability affecting the off-chain components of the [Across](https://across.to/) bridge and reported it to Risk Labs. No user funds were at risk, but the vulnerability could have allowed malicious actors to trigger a double-spend condition by having their bridge deposits fulfilled **twice** by Across’ relayers. The vulnerability was identified and reported shortly after its introduction to the GitHub repository. Risk Labs and Across promptly took steps to pause the relayers and fix the problem. Risk Labs awarded iosiro with a generous bounty of $90,000 for their submission.
## Across Bridge basics
*The processes described here were derived from the relayer-v2 repository at commit [7f9ef4b979ede6e1e9ff3fdf043f536df4871243](https://github.com/across-protocol/relayer-v2/tree/7f9ef4b979ede6e1e9ff3fdf043f536df4871243).*
Across protocol allows users to bridge funds between various chains in a decentralized manner. Anyone can become a Relayer, and earn fees by fulfilling cross-chain deposits. After deposits have been filled, the Across Dataworker will refund Relayers. In the event that Relayers cannot fill deposits, the Dataworker can be signaled to perform a "slow relay", filling the deposit with liquidity sourced from liquidity providers.
### Relayers
When users want to transfer funds from a source chain to a destination chain, they do so by calling the `deposit()` function in the `Spokepool` contract on the source chain. The function accepts various arguments, including the deposit's destination chain, as well as a percentage fee that the depositor is willing to pay any Relayer willing to process their transaction. The funds are pulled into the `Spokepool` contract, and a `FundsDeposited` event is emitted.
Both the Relayer and Dataworker clients monitor for these on-chain events via the off-chain Spokepool client. The Relayer client will, at regular intervals, iterate over all detected and unfilled deposits to do one of three things:
- If the relay is considered profitable given the attached fee percentage, the Relayer has sufficient funds on the destination chain, and the deposit has not received any prior fills, the Relayer will fill the deposit by calling `Spokepool::fillRelay()` on the destination chain.
- If the relay is not considered profitable, skip it. Users can update the fee percentage of their unfilled deposits by calling `Spokepool::speedUpDeposit()`.
- If the relay is considered profitable, but the Relayer does not have sufficient funds on the destination chain, it will trigger a slow refill by calling `Spokepool::fillRelay()` on the destination bridge, specifying a fill amount of `1 wei`.
In the third scenario, both the Relayer and Dataworker clients will be notified of the fill by the Spokepool client, through the `FilledRelay` on-chain event. The Relayer will, however, ignore the deposit in the future, as it had already received a fill, but the Dataworker will use this as a signal to complete the deposit.
### Deposits vs. fills
Care must be taken when identifying deposits and fills, both on-chain and off-chain, to ensure that:
1. Off-chain clients can accurately tie fills to deposits
2. In the event that a deposit is handled incorrectly off-chain, on-chain controls will prevent erroneously bridging more funds than necessary.
Across's design is sound in this regard. Various controls across the codebase ensure the integrity of the deposit lifecycle, while still maintaining a permissionless relayer model.
### Off-chain
Deposits and fills are defined [here](https://github.com/across-protocol/relayer-v2/blob/7f9ef4b979ede6e1e9ff3fdf043f536df4871243/src/interfaces/SpokePool.ts#L5). The function [validateFillForDeposit](https://github.com/across-protocol/relayer-v2/blob/7f9ef4b979ede6e1e9ff3fdf043f536df4871243/src/clients/SpokePoolClient.ts#L202) determines whether a specific fill (recorded via the `FillRelay` on-chain event) matches a specific deposit as shown below.
<pre>
<code class="language-javascript">
validateFillForDeposit(fill: Fill, deposit: Deposit) {
let isValid = true;
Object.keys(deposit).forEach((key) => {
if (fill[key] !== undefined && deposit[key].toString() !== fill[key].toString()) isValid = false;
});
return isValid;
}
</code>
</pre>
This function iterates over the properties of a deposit and a fill to ensure that they match. Should any properties differ, the pair will be considered invalid. These properties include `depositId`, `destinationChainId`, `relayerFeePct`, `recipient`, etc.
The Relayer client can use this functionality to determine whether a deposit has already received a fill, as they are only meant to act on deposits that haven't had any fills. Similarly, the Dataworker client can use this functionality to identify a fill that signals a slow relay.
### On-chain
Remember that relaying is permissionless and Relayers operate independently from one another. So what prevents two Relayers from filling the same deposit twice, sending the recipient funds twice, and thereby triggering multiple refunds from the Dataworker? Simple! The `Spokepool::fillRelay()` function starts by calculating a hash of the deposit information on the destination chain, shown below. Notice `destinationChainId` being forcefully set to `chainId()`? Sorry sir, no replay attack for you.
<pre>
<code class="language-solidity">
SpokePoolInterface.RelayData memory relayData = SpokePoolInterface.RelayData({
depositor: depositor,
recipient: recipient,
destinationToken: destinationToken,
amount: amount,
realizedLpFeePct: realizedLpFeePct,
relayerFeePct: relayerFeePct,
depositId: depositId,
originChainId: originChainId,
destinationChainId: chainId()
});
bytes32 relayHash = _getRelayHash(relayData);
</code>
</pre>
It then uses the hash in the internal function `Spokepool::_fillRelay()` to ensure that the deposit is not being filled beyond the original deposit amount and then increments the fill amount with the amount being bridged.
<pre>
<code class="language-solidity">
// ensure deposit has not been filled yet
require(relayFills[relayHash] < relayData.amount, "relay filled");
// bunch of code to determine transfer amount
// increase the amount that has already been filled
relayFills[relayHash] += fillAmountPreFees;
</code>
</pre>
Roughly speaking, if two Relayers were to attempt to fully fill the same deposit at the same time, the second Relayer's transaction would revert. In short, the above implementation prevents on-chain double spending, with the nice side-effect of enabling partial fills.
### Refunds and slow relays
Refunds and slow relays are handled by the Dataworker client. Whenever a Relayer fills a deposit, the Dataworker will ensure that the Relayer gets refunded, and whenever a slow relay is triggered, the Dataworker will ensure that the deposit is filled (acting as a Relayer, but sourcing the funds from the `Spokepool` contract itself). To achieve this, the Dataworker builds merkle trees for both refunds and slow relays. The roots of these trees are proposed via the `Hubpool` contract, and can be disputed. When the dispute window is over, the roots can be executed, which will update the `Spokepool` contract with the new refund and slow relay roots.
With these new roots, the `Spokepool::executeRelayerRefundLeaf()` and `Spokepool::executeSlowRelayLeaf()` functions can be called to redeem a refund or complete a slow relay, provided the caller has a valid proof for their claim.
### Deposit storage
The Spokepool client is responsible for monitoring events emitted by the `Spokepool` contract. New deposits are stored in three different objects:
- `deposits`: Uses the `destinationChainId` as a key to map an array of deposits to that chain
- `depositsWithBlockNumbers`: Also uses `destinationChainId` as a key to map to an array of deposits, but includes additional information, such as the block number in which the deposit was made. This is used by the Relayer client to ignore deposits beyond a certain age.
- `depositHashes`: Maps a generated key, comprised of the `depositId` and the `originChainId`, to a specific deposit. This is used by the Dataworker client to find deposits given a specific fill (note that this is prior to in-depth validation, which is still performed).
As the Dataworker client primarily uses `depositHashes` for deposit information, and the Relayer client uses `depositsWithBlockNumbers`, these objects need to be consistent with each other, as any discrepancies in the deposit information could lead to different results when calling `validateFillForDeposit()`. The bug that we reported led to exactly this condition.
## Bug details
The Relayer client, through [checkForUnfilledDepositsAndFill()](https://github.com/across-protocol/relayer-v2/blob/7f9ef4b979ede6e1e9ff3fdf043f536df4871243/src/relayer/Relayer.ts#L23), will attempt to fill deposits based on a number of criteria. Deposits to be filled are obtained by calling [getUnfilledDeposits()](https://github.com/across-protocol/relayer-v2/blob/7f9ef4b979ede6e1e9ff3fdf043f536df4871243/src/relayer/Relayer.ts#L27), which then calls [getValidUnfilledAmountForDeposit()](https://github.com/across-protocol/relayer-v2/blob/7f9ef4b979ede6e1e9ff3fdf043f536df4871243/src/clients/SpokePoolClient.ts#L175). `getValidUnfilledAmountForDeposit()`, crucially, returns `fillCount` (the number of fills that exist for the deposit) and `unfilledAmount` (the total number of tokens that still need to be deposited/relayed.
`fillCount` is used to determine when the Relayer client should initiate a slow relay if the Relayer client does not have a sufficient balance ([Relayer.ts#L75](https://github.com/across-protocol/relayer-v2/blob/7f9ef4b979ede6e1e9ff3fdf043f536df4871243/src/relayer/Relayer.ts#L75)). If `fillCount != 0`, we can assume that a fill has already started and the Relay client can ignore the deposit. Similarly, `unfilledAmount` ensures that a deposit that has already been filled does not get included in the list of unfilled deposits for processing ([FillUtils.ts#L183](https://github.com/across-protocol/relayer-v2/blob/7f9ef4b979ede6e1e9ff3fdf043f536df4871243/src/utils/FillUtils.ts#L183)).
These checks should ensure that the Relayer client does not attempt to process deposits already filled by the Dataworker.
However, these restrictions can be bypassed under two conditions:
1. The function [getValidUnfilledAmountForDeposit()](https://github.com/across-protocol/relayer-v2/blob/7f9ef4b979ede6e1e9ff3fdf043f536df4871243/src/clients/SpokePoolClient.ts#L175) correctly obtains the previous fills for the deposit ([SpokePoolClient.ts#L176](https://github.com/across-protocol/relayer-v2/blob/7f9ef4b979ede6e1e9ff3fdf043f536df4871243/src/clients/SpokePoolClient.ts#L176)), but when validating that the fills match the deposit ([SpokePoolClient.ts#L181](https://github.com/across-protocol/relayer-v2/blob/7f9ef4b979ede6e1e9ff3fdf043f536df4871243/src/clients/SpokePoolClient.ts#L181)), it is done against a deposit originating from the `depositsWithBlockNumbers` object, which can be changed during a deposit speed up. The `depositsWithBlockNumbers` object is obtained during a call to `getUnfilledDeposits()`, by calling [getDepositsForDestinationChain()](https://github.com/across-protocol/relayer-v2/blob/7f9ef4b979ede6e1e9ff3fdf043f536df4871243/src/utils/FillUtils.ts#L166) with `withBlock = true`.
2. The `SpokePool.sol` contract uses a hash of the relay data ([SpokePool.sol#L374](https://github.com/across-protocol/contracts-v2/blob/9656a972da602243943bd7e20c06545daa3fbfcb/contracts/SpokePool.sol#L374)) to track on-chain the amounts that have been filled for a specific deposit, but includes `relayerFeePct` in the hash. This value can be changed when speeding up deposits.
The core issue that was identified was that the `relayerFeePct` field would be updated for a sped up deposit within the `depositsWithBlockNumbers` object. When processing a `RequestedSpeedUpDeposit` event, the Spokepool client would call [appendMaxSpeedUpSignatureToDeposit](https://github.com/across-protocol/relayer-v2/blob/7f9ef4b979ede6e1e9ff3fdf043f536df4871243/src/clients/SpokePoolClient.ts#L156) (shown below), and if the new `relayerFeePct` was higher than the old `relayerFeePct`, it would return a deposit with an updated `relayerFeePct` field.
<pre>
<code class="language-javascript">
appendMaxSpeedUpSignatureToDeposit(deposit: Deposit) {
// omitted
return { ...deposit, speedUpSignature: maxSpeedUp.depositorSignature, relayerFeePct: maxSpeedUp.newRelayerFeePct };
}
</code>
</pre>
This update would then be written to the `depositsWithBlockNumbers` object [SpokePoolClient.ts#L390](https://github.com/across-protocol/relayer-v2/blob/7f9ef4b979ede6e1e9ff3fdf043f536df4871243/src/clients/SpokePoolClient.ts#L390), as shown below.
<pre>
<code class="language-javascript">
if (eventsToQuery.includes("RequestedSpeedUpDeposit")) {
const speedUpEvents = queryResults[eventsToQuery.indexOf("RequestedSpeedUpDeposit")];
for (const event of speedUpEvents) {
const speedUp: SpeedUp = { ...spreadEvent(event), originChainId: this.chainId };
assign(this.speedUps, [speedUp.depositor, speedUp.depositId], [speedUp]);
}
// Traverse all deposit events and update them with associated speedups, If they exist.
for (const destinationChainId of Object.keys(this.depositsWithBlockNumbers))
for (const [index, deposit] of this.depositsWithBlockNumbers[destinationChainId].entries()) {
const { blockNumber, originBlockNumber, ...depositData } = deposit;
const speedUpDeposit = this.appendMaxSpeedUpSignatureToDeposit(depositData);
if (speedUpDeposit !== depositData) {
this.deposits[destinationChainId][index] = speedUpDeposit;
this.depositsWithBlockNumbers[destinationChainId][index] = {
...speedUpDeposit,
blockNumber,
originBlockNumber,
};
}
}
}
</code>
</pre>
## Exploitation
In order to exploit this issue, an attacker would essentially need to start by transferring funds from one chain to another. However, to ensure active Relayer clients do not interact with the deposit (yet), the attacker would immediately trigger a slow relay themselves, by calling `Spokepool::FillRelay()` on the destination bridge. This would
- inform any Relayer clients that a fill is in progress, and they need not do anything with it, and;
- signal the Dataworker client to perform a slow relay.
Following this, the attacker would then update their deposit's `relayerFeePct` by calling the `Spokepool::speedUpDeposit()` function on the source chain. This will cause an update to the `depositsWithBlockNumbers` deposit, such that the original slow relay signal, as well as the slow relay itself, will be discounted as valid fills, triggering an additional relay of funds.
## Remediation
This issue was remediated by adding additional client-side properties for deposits that were sped up to ensure that clients could accurately distinguish between deposits that have been filled or not, regardless of their updated `relayerFeePct`.
## Conclusion
We’d like to give a big thanks to Risk Labs for their responsiveness and support during the disclosure process, as well as running a public bug bounty program.
Smart contract audits are generally the primary security service delivered in the Web3 space. However, many projects (especially bridges) rely heavily on off-chain components that often expose a tremendous amount of risk and might not be in-scope for traditional audits or bug bounty programs. We’d like to use this as an opportunity to highlight that Web2 technology still underpins aspects of the Web3 world, and can benefit from regular, in-depth security reviews and bug bounties.
If you’d like us to take a look at your protocol’s off-chain components, reach out to us at hello@iosiro.com.
