Post-Judging QA

[liveactionllama]

1. Please post feedback about specific findings on the relevant issue in this repo.
2. More general feedback for the judge can be shared here.

The judge for this contest is @0xleastwood.

Reminders

• Please keep your comments as fact-based as possible.
• C4 judges have final authority in determining validity and severity.
• You are welcome to start new discussions here!
• Please link to specific issues wherever relevant.

Thank you!

[JeffCX]

With full respect to @0xleastwood expertise,

I left my comments on finding

#72

#69

#47

I fully respect judge's final decision and will have no further dispute!

Thanks!

[JeffCX]

left my comment for

#114

I fully respect judge's final decision and will have no further dispute!

Thanks!

[GalloDaSballo]

From my own understanding, I believe that the findings left open are not valid HMs, for the following reasons:

Highs

Disputing the 28_750_517 report as Low Severity

#72

Discussion around this issue
sherlock-audit/2023-03-optimism-judging#85

Context around maximum used gas under normal circumstances
sherlock-audit/2023-03-optimism-judging#93 (comment)
I had fetched all bridge tx from past 6 months and the only few outliers still consume at most 3MLN gas

Ultimately no normal withdrawal would be bricked unless the end user sets the gas to an abnormal value

Disputing Chain split caused by memory corruption in EVM

#56

Disputing due to lack of proof, if you check latest on GETH , op GETH uses the same code:
https://github.com/ethereum/go-ethereum/blob/8bbb16b70ef4a9e732745aac104525a6f16306d6/core/vm/instructions.go#L695

Meaning that the lack of using ret = common.CopyBytes(ret) is not sufficient for the bug

Disputing as invalid

Permissionless block user's withdrawal by taking advantage of the reentrancy protection in CrossDomainMessager.sol
#119

If you enter the early return check, with sufficient gas, you’ll store the message as failed, and ETH will remain the XDomainMessenger

You’ll then be able to replay from there

Calling again from the portal would revert at the second call from the Portal, because the message would already be set as failed, since it’s failed, it won’t be repayable additional times

MEDS


Lack of message expiration - Disputing as QA Refactoring

#118

The bridge and messages are not meant to be time sensitive, but they are meant to be repayable

The finding speaks around what could be considered a feature request more so than a vulnerability

Additionally, in the case of pausing, the xDomainMessenger does offer deployability guarantees meaning that tokens transferred wouldn’t be lost

Alternative use cases can be constructed but a lack of deadline doesn’t demonstrate a risk nor a vulnerability for the contracts in scope

Approving Bridge and xDomainMessenger - Disputing as Self-Rekt

#114

By the same token approving a malicious wallet, or transferring tokens to address(0) for tokens that have permit with no 0 check, are not considered Medium vulnerabilities

Ultimately the approval would have to be self-inflicted since the normal path doesn’t ever require giving approval to those contracts, nor such an approval can be faked

Dictator - Disputing as Low and OOS

#49

There are 2 aspects to this report:

1. That because of previous reports, we know that the comment is the incorrect aspect, not the functionality, meaning that the finding is best categorise as incorrect spec / Low severity
2. The file is out of scope, we spoke with the Base Team and know that they will not perform a migration (they will start from genesis), additionally the file and the folder are not in scope nor any indication was given for them to be added to the scope (since they will not perform a migration)

Condition for Revert - Disputing as incorrect

#47

The finding asserts that the Portal should revert on failure, but that’s not how the Portal is mean to be used, the Portal ignores success and Replayability is guaranteed by xDomainMessenger and other “closer to the user contracts”

The Portal is only meant to perform the operations once, and the revert is means to be exclusively for gas tracking, meaning that the check is meant to be a && and not the suggested change

I have omitted #124 because while I believe the math is incorrect, I want to double check it

The issue with the POC in #124 is that is simply demonstrates how to leave ETH in the xDomainMessenger, for a message than can be replayed, which is intended usage since anybody can relay the message successfully

[JeffCX]

For #119

If you enter the early return check, with sufficient gas, you’ll store the message as failed, and ETH will remain the XDomainMessenger

You’ll then be able to replay from there

Calling again from the portal would revert at the second call from the Portal, because the message would already be set as failed, since it’s failed, it won’t be repayable additional times

Practically,

the attacker can help user choose finalizeWithdrawal or relayMessenge

user definitely expect to receive ETH after finalizeWithdrawal

the attacker force either protocol or the user to call relayMessage themselves, which is not expected by protocol and user.

https://docs.code4rena.com/awarding/judging-criteria/severity-categorization

2 — Med: Assets not at direct risk, but the function of the protocol or its availability could be impacted, or leak value with a hypothetical attack path with stated assumptions, but external requirements.

I think this fails into "availability could be impacted"

Imagine user bridge 10 transactions

attack randomly pick up 3 and help user call finalizeWithdrawal,

user has to call relayMessenge themselves 3 times

then bridge 10 transactions

attack randomly pick up 6 and help user call finalizeWithdrawal,

user has to call relayMessenge themselves 6 times

AND

In the testnet:

https://bridge.base.org/withdraw

only finalizeWithdrawal is supported, not relayMessage, so such issue can greatly impact the availability and usability of the protocol

[JeffCX]

And feel like the issue #72 and #114 are tied together, both requires user error first

#114 (comment)

there should be documentation to explicitly warn user not set the gas limit exceed block gas limit

there should be documentation to explicitly warn user not giving approval (the cross-chain message support arbitrary call, arbitrary call means support anything, I think giving approval is such a normal things in defi for composability reason, without proper warning, user can rekt themselves)

[JeffCX]

#118 In regards to lack of expiration in cross chain messaging

There are actually issue around this,

sherlock-audit/2023-05-ecoprotocol-judging#142

the protocol integrate with the cross-domain message and a user can queue a transaction A, wait for transaction B to executed, then execute a the transaction A to overwrite the state of transaction B

this does requires a certain pre-condition that the message needs order of delivery

Pretty much the same issue as "signature and approval never expires " :)

[GalloDaSballo]

For #119 the frontrunner is paying 120k+ gas (40k + 80k to pass all checks), for it to revert
The message and value are then left in xDomainMessenger and nobody can deny that tx

The cost for the revert to happen to the end user is closer to 40k gas, they can then finalize their withdrawal via the xDomainMessenger

The UI is not in scope

---

Agree on docs (Low Severity)

---

#118 The Optimism team has built credibly neutral infrastructure, adding expiration at the base level adds MEV for block producers / sequencer (it creates incentives to censor txs)

The linked issue from Sherlock has to do with a mistake the integrator did in assuming that each relayed message is the last, and should be viewed as a bug in the integrator code, not OP, since the integrator could build a resilient system without needing to change the OP codebase

---

[GalloDaSballo]

Have computed the Math related to #124 with one the highest rated Watons and we have agreed, that due to the extra overheads in hasMinGas the call always has at least 25k * 63/64 gas, which makes any minor incorrect gas estimate at the xDomainMessenger not sufficiently big.

The only case where the gas could have been tight was if the xDomainMessenger was used to send ETH to a new account (which incurs an additional 25k gas cost), however, even in that case, the extra overhead offered by the Portal is sufficient

[0xleastwood]

@GalloDaSballo I appreciate all the additional context. To clarify, these issues were left as valid HMs until the protocol team could verify the POC. There was a bit of urgency to go through the judging process as the sponsor will not be available soon.

[0xleastwood]

I will go through these findings given the context provided by all the wardens and make my final judgements on the outcome:)

[Nadinnnn]

Hi @0xleastwood, hope you are doing well.
As I know there is no reward for Gas optimization in Base contest.

[Bauchibred]

Hi @0xleastwood, thanks for your work and time!

I've provided more context to my argument for #30. I respect you and your expertise and will accept your final decision after your review of the additional information. Thank you.

[sathishpic22]

Hi @0xleastwood , thank you for your work.

My comments not yet checked. Please check my issue: #16

I have remind you 2 times. The post QA ends a week ago.

Thank you,