feat(contract-check): static checks for encrypted-type boundary rules - #341
Draft
alexshchur wants to merge 6 commits into
Draft
feat(contract-check): static checks for encrypted-type boundary rules#341alexshchur wants to merge 6 commits into
alexshchur wants to merge 6 commits into
Conversation
Reads solc build-info (hardhat or foundry) and reports violations of the
shared-euint boundary conventions:
no-raw-encrypted-params external/public must not accept raw euint*/ebool/
eaddress (recurses through structs and arrays)
no-raw-encrypted-returns external/public state-mutating must not return raw
encrypted types; view/pure exempt
no-raw-shared-wrap sharedE*.wrap/unwrap only inside the FHE library
proof-placement and receive-variant are registered but inert, pending the
calldata convention decision and data-flow support respectively.
Ships a library API plus a contract-check CLI; consuming build-info rather than
parsing Solidity means types arrive already resolved by the compiler.
Documents each rule with the reasoning and correct/incorrect examples, the recognised type inventory, and how to wire the check into hardhat/foundry projects, package scripts, CI, or code.
…ment opt-in
Splits the parked proof rule in two, since the library supports both a proof
per value and one signature over a batch:
external-input-missing-proof error. External inputs with no proof bytes can
never be converted to a usable handle, which is
wrong under either arrangement.
proof-placement warning, off by default. Set proofStyle to
'trailing' or 'per-value' to pin a house style,
typically to match a generated client encoder.
Exposed as a proofStyle option and a --proof-style flag. No findings against
local production build-info, so the error rule adds no false positives.
…ysis receive*Param verifies provenance against msg.sender; receive*FromCall against a named callee. Swapping them reverts opaquely at runtime, so the mismatch is worth catching statically. Origin is resolved only where locally provable — the argument is a call expression, a parameter reference, or a local assigned exactly once. Reassigned locals, storage reads and struct members yield unknown and the rule stays silent, so it reports what it can prove instead of guessing. Warning severity, since this is a usability failure rather than a security one. All five rules are now implemented; no findings against local production build-info.
Exact-pin devDependencies per the pinned-deps check, add lint/check:types scripts and eslint-config to match sibling packages, prettier-format all sources, and add the changeset. The real-build smoke test now reads CONTRACT_CHECK_FIXTURE instead of a hardcoded local path, so it skips cleanly everywhere it is not set.
🦋 Changeset detectedLatest commit: 04f77ed The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
1 Skipped Deployment
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Adds
@cofhe/contract-check: static checks that the encrypted-type boundary conventions are actually followed in contract signatures.Why
Encrypted values travel as handles, and a contract is normally ACL-allowed on the handles it works with. So a function accepting a raw
euint64from outside can be induced to compute on someone else's ciphertext using its own authority — the confused-deputy shape behind the confidential-deposit finding. ThesharedE*/externalE*types close that hole by making a live handle unrepresentable at a trust boundary, but only if signatures use them. Nothing enforced that until now.Rules
no-raw-encrypted-paramsno-raw-encrypted-returnsview/pureexemptno-raw-shared-wrapsharedE*.wrap/.unwraponly inside the FHE libraryexternal-input-missing-proofproof-placementreceive-variantreceive*Paramvsreceive*FromCallmust match the value's originTwo design notes worth reviewing:
proof-placementis off by default.FHE.asEuintXX(hash, proof)and the batch verifier are both first-class, so neither arrangement is a defect and the checker accepts both. SetproofStyle: 'trailing' | 'per-value'only to pin a house style — typically so signatures match a generated client encoder. Deviations warn; they never fail a build.receive-variantreports only what it can prove. Origin is resolved when the argument is a call expression, a parameter reference, or a local assigned exactly once. Reassigned locals, storage reads and struct members yield "unknown" and the rule stays silent, rather than guessing with a heuristic.How it works
Reads solc build-info (Hardhat
artifacts/build-info, Foundryout/build-infowith--build-info) rather than parsing Solidity. The compiler has already resolved imports, aliases and inheritance, soeuint64is unambiguous no matter how it was imported. No new parser to maintain.Also exported as a library (
checkBuildInfo,checkBuildInfoFile,checkBuildInfoDir).Open question for reviewers
Run against a real token project today and you get a large report — the confidential-token base contracts and
IERC7984itself currently take and return raw handles at public boundaries. Locally that is ~117 findings, almost all from vendored sources rather than app code.That is the tool working correctly, but it means we need a policy before this can gate anything:
Currently only
FHE.soland thecofhe-contractspath are exempt, overridable vialibraryPaths. Which of the three we want determines what the config surface should look like.Draft because
excludeglobs + per-rule severity) — that is the piece that makes this adoptable in a repo with vendored contracts, and its shape depends on the question abovereceive-variantcannot see through helper functions or reassigned locals; a Slither detector would close that gap later25 tests, no findings against real production build-info for the four newer rules.