fix: Solidity move basic-oracle-manipulation test annotation to the matched line - #3997
Merged
malaverdiere merged 1 commit intoJul 29, 2026
Merged
Conversation
malaverdiere
changed the title
…nary precedence The updated Solidity tree-sitter grammar correctly parses `cond ? x : y.div(z)` as `cond ? x : (y.div(z))` (member access binds tighter than the ternary operator). The basic-oracle-manipulation finding now points at the actual `.div` call line rather than the start of the enclosing ternary, so move the `ruleid` annotation to the corrected line. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
malaverdiere
force-pushed
the
marc-andre/fix-oracle-manipulation-solidity-line
branch
2 times, most recently
from
July 28, 2026 22:01
9f1890d to
f2b6518
Compare
2 tasks
kopecs
approved these changes
Jul 29, 2026
Contributor
|
Actually @malaverdiere you may need to make this change in https://github.com/semgrep/semgrep-rules-jsonnet to have it be properly handled |
Contributor
Author
That repo does not have a |
malaverdiere
deleted the
marc-andre/fix-oracle-manipulation-solidity-line
branch
July 29, 2026 23:47
jmgrosen
pushed a commit
to semgrep/semgrep
that referenced
this pull request
Aug 10, 2026
…proprietary#6563) What: Updates the `semgrep-solidity` tree-sitter grammar submodule (LANG-207) and adapts the OCaml CST-to-generic mapping (`Parse_solidity_tree_sitter.ml`) to the new grammar. Also bumps `semgrep-rules` for the matching Solidity test annotation fix. Pinned to the regenerated parser from [semgrep/ocaml-tree-sitter-semgrep#605](semgrep/ocaml-tree-sitter-semgrep#605) / [semgrep/semgrep-solidity#4](semgrep/semgrep-solidity#4) (`2f369b5`, tree-sitter **0.26.3**), and [semgrep/semgrep-rules#3997](semgrep/semgrep-rules#3997). New language support: - **Transient storage** (EIP-1153): `transient` state location, `tload`, `tstore` - **Named mapping parameters**: `mapping(address owner => uint256 balance)` - **Storage layout specifier** (0.8.29): `contract C layout at <expr>` - **Assembly flags**: `assembly ("memory-safe") { ... }` - **`global` using-directives** and **braced using-aliases**: `using {f, g} for T` - **Top-level events and using-directives** - **EVM Cancun builtins**: `mcopy`, `blobhash`, `blobbasefee`, `basefee`, `blobfee`, `prevrandao` Removed (not valid Solidity): `>>>`, `>>>=`, `!==`, unary `+` Behavioral fix: member access now binds tighter than the ternary `?:` operator. Previously `cond ? x : y.div(z)` parsed as `(cond ? x : y).div(z)`; it now correctly parses as `cond ? x : (y.div(z))`. Why: Bring Semgrep's Solidity parser in line with current Solidity / tree-sitter-solidity so newer language features parse correctly and ternary vs member-access precedence matches the language (LANG-207). Test plan: - [x] `make core` builds clean (tree-sitter 0.26.3) - [x] Solidity parsing fixtures: 13/13 pass - [x] Solidity-filtered OSS tests (`-s solidity`): 51 successful (50 pass, 1 xfail) - [x] `semgrep-rules` pin includes the `basic-oracle-manipulation` annotation fix for the precedence change synced from Pro 4d3b802b1d97220fe51caf37895a5f72f5981036
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
3 participants
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.
Relates to the Solidity grammar update from https://github.com/semgrep/semgrep-proprietary/pull/6721 and others..
Summary
The
basic-oracle-manipulationtest'spattern: $X.div($Y)match forunderlyingUnit.mul(balanceWithInvested()).div(totalSupply())is textually on the:(else) branch line of a multi-line ternary, not the line where the enclosing assignment starts. The// ruleid:annotation was placed one line above the wrong branch.Confirmed by parsing the snippet directly with tree-sitter-solidity v1.2.13 (via semgrep/ocaml-tree-sitter-semgrep#626):
The
.div(...)call is entirely on the:branch's line — there's no way for a textually-accurate match to be reported one line earlier. This surfaced as a CI failure (1 false negative, 1 false positive) in semgrep-proprietary#6721, which bumps tree-sitter-solidity to v1.2.13 and produces a more precise AST for this multi-line ternary than whatever it replaced.Change
Moves the
// ruleid: basic-oracle-manipulationannotation from immediately above the assignment to immediately above the actual matched (:) branch. No line count change, so no ripple to other annotations later in the file.Test plan
basic-oracle-manipulation.yaml).🤖 Generated with Claude Code