Skip to content

security-reviews: Add 3Doc report on Canon F_GETFD changes. #12960

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
ajsutton merged 2 commits into from
Nov 19, 2024
Merged

security-reviews: Add 3Doc report on Canon F_GETFD changes. #12960

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
f8a32bd
security-reviews: Add 3Doc report on Canon F_GETFD changes.
ajsutton Nov 19, 2024
e444e4c
security-reviews: Update README.
ajsutton Nov 19, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
114 changes: 114 additions & 0 deletions docs/security-reviews/2024_10-Cannon-FGETFD-3DocSecurity.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,114 @@
# Audit Report - OP Cannon

| | |
| -------------- | ------------------------------------------------------------------------- |
| **Audit Date** | Oct 2nd 2024 - Oct 3rd 2024 |
| **Auditor** | 3DOC Security ([@3docSec](https://x.com/3docSec)) |
| **Version 1** | Oct 3rd 2024. |

<br clear="both" />

# Contents
- [Audit Report - OP cannon](#audit-report---op-cannon)
- [Contents](#contents)
- [Disclaimer](#disclaimer)
- [About 3DOC](#about-3doc)
- [Scope](#scope)
- [Severity Classification](#severity-classification)
- [Summary](#summary)
- [Findings](#findings)
- [Low Risk Findings (1)](#low-risk-findings-1)
- [1. Op-challenger Docker image does not include Cannon embeds](#-op-challenger-docker-image-does-not-include-cannon-embeds)

# Disclaimer
_The following audit report is based on the information and code provided by the client, and any findings or recommendations are made solely on the basis of this information. While the Auditor has exercised due care and skill in conducting the audit, it cannot be guaranteed that all issues have been identified and that there are no undiscovered errors or vulnerabilities in the code._

_Furthermore, this report is not an endorsement or certification of the protocol, and the Auditor does not assume any responsibility for any losses or damages that may result from the use of the smart contracts, either in their current form or in any modified version thereof._

# About 3DOC
3DOC is a top ranked Smart Contract Auditor doing audits on Code4rena (www.code4rena.com), having ranked 1st in multiple contests in [solo](https://code4rena.com/@3docSec) and [team](https://code4rena.com/@RadiantLabs) audits, including the [Optimism superchain contest](https://code4rena.com/audits/2024-07-optimism-superchain) in July 2024.<br>
He can also be booked for conducting Private Audits.

Contact: <br>

X: [@3DocSec](https://x.com/3DocSec)

e-mail: [hello@3doc.fr](mailto:hello@3doc.fr)

# Scope
The scope of the audit is the following Pull Request in the client's GitHub repository:

https://github.com/ethereum-optimism/optimism/pull/12050

The change consists of a core update for supporting the `F_GETFD` syscall in the MIPS VM, [provided with this commit](https://github.com/ethereum-optimism/optimism/pull/12050/commits/7c8257d3574a2a76ab90f8129c7b532d68049944), and several additional updates accommodating the VM version bump that came with the core change.

# Severity Classification
| Severity | Impact: High | Impact: Medium | Impact: Low |
| ---------------------- | ------------ | -------------- | ----------- |
| **Likelihood: High** | ![high] | ![high] | ![medium] |
| **Likelihood: Medium** | ![high] | ![medium] | ![low] |
| **Likelihood: Low** | ![medium] | ![low] | ![low] |

**Impact** - the technical, economic and reputation damage of a successful attack

**Likelihood** - the chance that a particular vulnerability is discovered and exploited

# Summary

| Severity | Total |
| -------------- | ----- |
| ![high] | 0 |
| ![medium] | 0 |
| ![low] | 0 |
| ![information] | 0 |


# Findings
## Low Risk findings (0)

### [False positive] Op-challenger Docker image does not include Cannon embeds
#### Description
The change in scope added a new implementation of the Cannon VM, which was called `VersionSingleThreaded2`. Cannon has now three versions (`VersionSingleThreaded`, `VersionSingleThreaded2`, and `VersionMultiThreaded`).

The op-challenger program makes use of the Cannon VM in several places via the configured `VmBin` path, which point to the `multicannon` command line. This one reads the State version from the input state and selects the right Cannon VM accordingly (`cannon/multicannon/exec.go:L81`).

If we look at the Docker challenger image generated by the `make golang-docker` command, however, we can see it doesn't contain an `embeds` folder:

```
docker run -t us-docker.pkg.dev/oplabs-tools-artifacts/images/op-challenger find / -name embeds
```

But it however has the `cannon` command pointing to the `multicannon` multiplexer:

```
➜ optimism git:(52d0e60c1) ✗ docker run -t us-docker.pkg.dev/oplabs-tools-artifacts/images/op-challenger cannon | head -2
NAME:
multicannon - MIPS Fault Proof tool
➜ optimism git:(52d0e60c1) ✗
```

This issue appears to be pre-existing to the changes in scope; using Docker images to run the challenger is [mentioned as option](https://docs.optimism.io/builders/chain-operators/tools/op-challenger), but only as alternative option, hence the Low risk assessed for this finding.

#### Impact
Because of this issue, challenger instances operated in a Docker container won't be able to function properly.

#### Recommendation
Consider modifying the Docker build chain to include the `embeds` folder.
Consider extending the current e2e test suite to cover execution from Docker images.

#### Discussion

> @inphi The cannon-2 implementation that supports go1.22 is now embedded into the cannon cli binary. Note that these embeds are not actual files that you can find in the docker container filesystem. But rather an embedded filesystem inside the Go binary - https://pkg.go.dev/embed.

> @3DOC Oh yes I see that. So those are included in an embedded filesystem, I missed that


[high]: https://img.shields.io/badge/-HIGH-b02319 "HIGH"
[medium]: https://img.shields.io/badge/-MEDIUM-orange "MEDIUM"
[low]: https://img.shields.io/badge/-LOW-FFD700 "LOW"
[information]: https://img.shields.io/badge/-INFORMATION-darkgreen "INFORMATION"
[fixed]: https://img.shields.io/badge/-FIXED-brightgreen "FIXED"
[acknowledged]: https://img.shields.io/badge/-ACKNOWLEDGED-blue "ACKNOWLEDGED"
[disputed]: https://img.shields.io/badge/-DISPUTED-lightgrey "DISPUTED"
[reported]: https://img.shields.io/badge/-REPORTED-lightblue "REPORTED"
[partiallyfixed]: https://img.shields.io/badge/-PARTIALLY_FIXED-lightgreen "PARTIALLTY FIXED"
3 changes: 2 additions & 1 deletion docs/security-reviews/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ Each review is focused on a different part of the codebase, and at a different p
Please see the report for the specific details.

| Date | Reviewer | Focus and Scope | Report Link | Commit | Subsequent Release |
| ------- | -------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | ---------------------------------------------------------------------------------------------------------------------------------------------------------- | -------------------------------------------- | ------------------- |
|---------|----------------------| ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- |------------------------------------------------------------------------------------------------------------------------------------------------------------| -------------------------------------------- |---------------------|
| 2020-10 | Trail of Bits | Rollup | [2020_10-TrailOfBits.pdf](./2020_10-Rollup-TrailOfBits.pdf) | | |
| 2020-11 | Dapphub | ECDSA Wallet | [2020_11-Dapphub-ECDSA_Wallet.pdf](./2020_11-Dapphub-ECDSA_Wallet.pdf) | | |
| 2021-03 | OpenZeppelin | OVM and Rollup | [2021_03-OVM_and_Rollup-OpenZeppelin.pdf](./2021_03-OVM_and_Rollup-OpenZeppelin.pdf) | | |
Expand All @@ -27,5 +27,6 @@ Please see the report for the specific details.
| 2024-03 | Sherlock | Fault Proofs | Sherlock Optimism Fault Proofs Contest ([site](https://audits.sherlock.xyz/contests/205), [repo](https://github.com/sherlock-audit/2024-02-optimism-2024)) | | |
| 2024-08 | Cantina | Fault proof no-MIPS: All contracts in the `packages/contracts-bedrock/src/dispute` directory | [./2024_08_Fault-Proofs-MIPS_Cantina.pdf](./2024_08_Fault-Proofs-MIPS_Cantina.pdf) | 1f7081798ce2d49b8643514663d10681cb853a3d | op-contracts/v1.4.0 |
| 2024-08 | Spearbit | Fault proof MIPS: `MIPS.sol` | [./2024_08_Fault-Proofs-No-MIPS_Spearbit.pdf](./2024_08_Fault-Proofs-No-MIPS_Spearbit.pdf) | 71b93116738ee98c9f8713b1a5dfe626ce06c1b2 | op-contracts/v1.6.0 |
| 2024-10 | 3Doc Security | Fault proof MIPS: `MIPS.sol` | [./2024_10-Cannon-FGETFD-3DocSecurity.md](./2024_10-Cannon-FGETFD-3DocSecurity.md) | 52d0e60c16498ad4efec8798e3fc1b36b13f46a2 | op-contracts/v1.8.0 |

[kontrol]: https://github.com/ethereum-optimism/optimism/blob/876e16ad04968f0bb641eb76f98eb77e7e1a3e16/packages/contracts-bedrock/test/kontrol/README.md