Skip to content

Remove TOCTOU hazards by moving all checks into execute and renaming the Component methods #3960

@hdevalence

Description

@hdevalence

Describe the bug

Zellic observed many TOCTOU bugs in our ActionHandler impls. These all have a common cause: the aggressive use of check_stateful, which does parallel checks against a snapshot of chain state prior to any execution. The problem is that this separates an action's checks from its execution.

We can eliminate this bug class by simply not doing this.

Expected behavior

  • All code currently in check_stateful impls shall be moved into the execute methods, unless there is a simple AND LOCAL explanation for why it is safe to perform the check against historical state.
  • All intra-action consistency checks shall be removed. For instance, we will not be checking that a transaction has duplicate nullifiers, because we will handle each action in isolation.
  • The current execute method shall be renamed check_and_execute.
  • The current check_stateful method shall be renamed check_historical, and the docs shall be updated to note that it is run in parallel with a snapshot of the chain state some time prior to execution of the action, that it presents a TOCTOU hazard, and that it should be avoided in favor of check_and_execute unless it is known to be safe.

Metadata

Metadata

Assignees

Type

No type

Projects

Status

Done

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions