Skip to content
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.

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

Fault Proof Fixes #11503

Merged
merged 25 commits into from
Aug 16, 2024
Merged

Fault Proof Fixes #11503

merged 25 commits into from
Aug 16, 2024

Conversation

Inphi
Copy link
Contributor

@Inphi Inphi commented Aug 16, 2024

No description provided.

ajsutton and others added 25 commits August 16, 2024 09:58
FDGs with a split depth below 2 can trigger bugs in clock
extension. Since we don't expect to have a split depth anywhere
near 0 or 1 this is a low impact bug and doesn't have an impact
on production but should be prevented anyway.
Modifies the FDG constructor to correctly check that the
splitDepth +1 is gte the max game depth. Means that the splitDepth
is now limited to be 1 smaller than it was before. Fine in prod
but avoids a bug in the trace ancestor lookup logic.
Updates MIPSInstructions so that it correctly reverts on calls to
add, addi, and sub that overflow/underflow. Additionally includes
tests that demonstrates that the unchecked versions of the same
opcodes allow for overflow/underflow.
Existing implementation of SRAV had a bug where it would perform a
shift with all bytes of the rs register when the spec says it
should only be using the lower 5 bits of the register. Updates the
implementation to reflect this, updates the existing test to use
the same test vector as provided in the open mips tests, and adds
fuzz tests that shows srav works as expected with rs values that
have more than the lower 5 bits set.
Updates DelayedWETH to use call instead of transfer because
transfer only sends along 2300 gas which will cause the transfer
to fail if the owner contract uses lots of gas in the fallback
function. Adds tests that demonstrate that the function now works
appropriately under any reasonable amount of gas usage. In theory
this means the recovery function can be reentered but this isn't
an issue because the function is authenticated and is recovering
all of the ETH in the contract anyway.
Modifies the MIPS contracts to enforce that state.exited is either
exactly zero or one and cannot have any other value.
Existing step logic wrote the register offset into memory but the
compiler should already be doing this when the struct is defined.
Instead of writing directly into memory, this change verifies that
the memory at that location has the expected value which will
catch any cases where the compiler's memory allocation mechanism
changes in the future.
* contracts: Add gas input to precompile pre-images (#186)

Also update the cannon evm tests to use the new precompile preimage scheme.

---------

Co-authored-by: Adrian Sutton <[email protected]>

* op-challenger: Support uploading data in new format. (#188)

* op-program: Add required gas to precompile oracle key (#176)

* op-challenger: Support multiple versions of the preimage oracle contract

---------

Co-authored-by: Adrian Sutton <[email protected]>

---------

Co-authored-by: inphi <[email protected]>

---------

Co-authored-by: Adrian Sutton <[email protected]>
A lot of instructions in the MIPS spec require that certain fields
be set to zero. Most of the time this isn't actually a problem but
this can cause side-effects in a few cases. A buggy compiler could
create an issue if it ever spit out non-compliant instructions.
This PR implements those zero value enforcement checks for all
instructions that we implement.
* Prevent reiniting preimage uploads. (#190)

* fix(ctb): PreimageOracle `loadLocalData` length check

* fixes

* fix: bump preimage oracle semver

* chore: snapshots

* fix: gas snapshot

* revert: gas snapshot changes

---------

Co-authored-by: Adrian Sutton <[email protected]>
Co-authored-by: clabby <[email protected]>
* Prevent reiniting preimage uploads. (#190)

* fix(ctb): Perform correct clock validation in FDG constructor

Fixes the clock extension / max clock duration check in the
`FaultDisputeGame` constructor to account for the worst-case clock
extension.

* fix: add semver-lock

* fix: add kontrol

* gas snapshot

* fix: gas snapshot and semver-lock

---------

Co-authored-by: Adrian Sutton <[email protected]>
Co-authored-by: refcell <[email protected]>
* fix: no squeezing unfinalized proposals

Updates the PreimageOracle to correctly revert if a proposal has
not actually been finalized. This fixes a bug because the timestamp
is not set until after a proposal is finalized but all other
conditions in the squeeze function are met even before addLeaves
is called with the finalize boolean set.

* fix: ci checks

* fix: semver lock

---------

Co-authored-by: refcell <[email protected]>
When the FDG is about to execute a step, we must add an additional
clock extension equal to the PreimageOracle challenge period. This
gives the PreimageOracle time to elapse.
…279)

This reverts commit 864e59a821e87f9dd00ae97f85add93fa3857597.
This reverts commit 59e02fb3ece7fc249223054944a0b1ff437aa64f.
* cannon: Basic memory protections

Add memory protections against high memory allocations to the VM.
This prevents large allocations from causing the `heap` to overflow and
wrap into low memory, which could overwrite code and cause arbitrary code execution.

---------

Co-authored-by: inphi <[email protected]>
Creates a deploy script to deploy the implementations for the
FaultDisputeGame and PermissionedDisputeGame contracts being
updated as part of Granite. Based on the original FPACOPS script
with modifications to remove actions that are not necessary for
an upgrade.
Allow the `DeputyGuardian` to set the Anchor State for brick prevention.

---------

Co-authored-by: clabby <[email protected]>
Co-authored-by: refcell <[email protected]>
FPACOPS2 deployed a new AnchorStateRegistry proxy when the actual
script needed to just deploy a new implementation.
@Inphi Inphi marked this pull request as ready for review August 16, 2024 18:41
@Inphi Inphi requested review from a team as code owners August 16, 2024 18:41
@Inphi Inphi requested review from ajsutton and refcell August 16, 2024 18:41
@mslipper mslipper merged commit a81de91 into develop Aug 16, 2024
57 of 59 checks passed
@mslipper mslipper deleted the inphi/audit-fixes branch August 16, 2024 19:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants