code423n4 commented 1 year ago

Lines of code

https://github.com/code-423n4/2022-10-thegraph/blob/main/contracts/gateway/L1GraphTokenGateway.sol#L274-L276

Vulnerability details

Impact

According to the Arbitrum documentation,

"... Cross chain and cross shard interoperability is a hard problem, ...

It's important to differentiate between calls from Ethereum to Arbitrum and calls from Arbitrum to Ethereum. Ethereum contracts can send transactions into Arbitrum which arrive quickly. However general transactions from Arbitrum to Ethereum are slower since they need to wait for the challenge period to expire before being confirmed ..."

The challenge period is 7 days (for which the Ethereum blockchain cannot immediately confirm the correct state) to resolve any possible "fraud proof" that could arise between the asserter and the challenger, primarily hovering the four main types of censorship attacks: forking attacks, shunning attacks, jamming attacks, and speed demon attacks.

That aside, it is giving malicious attackers plenty of time to stealthily launch a series of small and yet sizable forgery of transactions that could fool the tree of assertions, allowing validators to pipeline execution by continuing to build the tree even before all nodes are confirmed via low level proof just like what recently happened to the BNB bridge on Oct 6, 2022.

Please refer to the following links for more details:

https://github.com/OffchainLabs/arbitrum/blob/master/docs/Rollup_basics.md https://github.com/OffchainLabs/arbitrum/blob/master/docs/Withdrawals.md https://globalcoinresearch.com/2021/06/29/a-comprehensive-analysis-of-arbitrum-security-mechanism/ https://rekt.news/bnb-bridge-rekt/

Proof of Concept

Now, to resolve confirmation delay, i.e L2 to L1 messages, notably, withdrawals, a Liquidity Exits mechanism is featured allowing users to experience L1 "Fast Withdrawals" by paying a third party liquidity provider a small fee on L2.

File: contracts/gateway/L1GraphTokenGateway.sol

274        // If the bridge doesn't have enough tokens, something's very wrong!
275        require(_amount <= escrowBalance, "BRIDGE_OUT_OF_FUNDS");
276        token.transferFrom(escrow, _to, _amount);

https://github.com/code-423n4/2022-10-thegraph/blob/main/contracts/gateway/L1GraphTokenGateway.sol#L274-L276

The critical issue lies on line 275 on the conditional check. And, as the comment suggested, something would be very wrong if the require statement ever reverted. All funds in the escrow would have been drained when this had happened, presumably due to the undetected malicious acts from the attackers.

Tools Used

Visual inspection and read up

Recommended Mitigation Steps

Consider refactoring the codes that would have the malicious attacks detected at the earliest possibility. To the least, an internal function that could validate the tally of L2 burn and L1 escrow release (using OpenZeppelin's ECDSA for instance) should be embedded before the require check.

trust1995 commented 1 year ago

can't extract useful insight from here because it is too generic.

0xean commented 1 year ago

closing as invalid and lacking specifics.

code-423n4 / 2022-10-thegraph-findings