Unified: Add control flow graph. - #7
anurag6569201 wants to merge 1 commit into
Conversation
Source PR: github#22479 Source head: f452bd4
⛔ Shipwright · BlockedRecommendation: do not merge PR #7 · Tier
Findings (7)
Fireworks usage: 11,803 input · 941 output · 12,744 total tokens · $0.0032 · 14s · 0 fix iteration(s) Open the Shipwright check for full evidence and the audit bundle. Use |
![shipwright-agent[bot]](https://avatars.githubusercontent.com/u/140162738?s=60&v=4){kind=small}
| } | ||
|
|
||
| class Parameter extends U::Parameter { | ||
| Expr getDefaultValue() { result = super.getDefault() } |
There was a problem hiding this comment.
Shipwright · CRITICAL
The 'Ast' module implementing 'AstSig<Location>' contains numerous stub classes with 'none()' bodies (e.g., 'ExprStmt', 'IfStmt', 'ForStmt', 'GotoStmt', 'Assignment', 'UntilStmt').
Impact: The 'Ast' module implementing 'AstSig<Location>' contains numerous stub classes with 'none()' bodies (e.g., 'ExprStmt', 'IfStmt', 'ForStmt', 'GotoStmt', 'Assignment', 'UntilStmt'). Any CFG construction that encounters these node types will silently produce no control-flow edges, yielding an incomplete or incorrect CFG. This can cause downstream analyses (e.g., taint, dataflow, dead-code) to miss paths and prod…
Suggested fix: Review the cited evidence, fix the risk if confirmed, and rerun Shipwright.
| class DefaultCase extends Case { | ||
| DefaultCase() { not exists(super.getPattern()) } | ||
| } | ||
|
|
There was a problem hiding this comment.
Shipwright · CRITICAL
'BooleanLiteral.getValue()' is implemented as 'result.toString() = super.getValue()'.
Impact: 'BooleanLiteral.getValue()' is implemented as 'result.toString() = super.getValue()'. This compares a string representation of a boolean result to the raw value, which is type-incompatible or always false depending on QL type resolution. If this compiles, it will never produce a true value, breaking CFG handling of boolean literals and any conditional logic depending on them.
Suggested fix: Review the cited evidence, fix the risk if confirmed, and rerun Shipwright.
| Expr getCondition() { result = super.getCondition() } | ||
| } | ||
|
|
||
| class UntilStmt extends LoopStmt { |
There was a problem hiding this comment.
Shipwright · HIGH
The 'Ast' module is a large hand-written adapter with many 'none()' stubs and TODO comments (e.g., '// TODO support foreach guard', '// TODO: sort out the relationship between Bina
Impact: The 'Ast' module is a large hand-written adapter with many 'none()' stubs and TODO comments (e.g., '// TODO support foreach guard', '// TODO: sort out the relationship between BinaryExpr and Assignment'). A new maintainer cannot determine which language constructs are actually supported by the CFG without exhaustively reading every stub. This is a maintainability hazard and likely to cause incorrect assumptions abou…
Suggested fix: Review the cited evidence, fix the risk if confirmed, and rerun Shipwright.
| } | ||
| } | ||
|
|
||
| /** A block statement. */ |
There was a problem hiding this comment.
Shipwright · HIGH
'Block.getLastStmt()' uses 'not exists(this.getStmt(i + 1))' to find the last statement.
Impact: 'Block.getLastStmt()' uses 'not exists(this.getStmt(i + 1))' to find the last statement. This relies on 'getStmt' indices being contiguous and starting at 0. If the underlying AST representation has gaps or non-zero-based indexing, this predicate will return no result or the wrong statement, and the intent is not documented.
Suggested fix: Review the cited evidence, fix the risk if confirmed, and rerun Shipwright.
| private predicate skipControlFlow(AstNode e) { e instanceof Modifier or e instanceof Identifier } | ||
|
|
||
| AstNode getChild(AstNode n, int index) { | ||
| result.getParent() = n and |
There was a problem hiding this comment.
Shipwright · HIGH
'getEnclosingCallable' recurses through parents but stops only when the parent is a 'Callable'.
Impact: 'getEnclosingCallable' recurses through parents but stops only when the parent is a 'Callable'. If an AST node is not enclosed by any callable (e.g., top-level declarations outside a 'TopLevel' body), the recursion can fail to produce a result or traverse unexpectedly, leaving CFG nodes without an enclosing callable and breaking scope-based queries.
Suggested fix: Review the cited evidence, fix the risk if confirmed, and rerun Shipwright.
| @@ -0,0 +1,40 @@ | |||
| /** | |||
There was a problem hiding this comment.
Shipwright · HIGH
The new 'printCfg.ql' query is tagged 'ide-contextual-queries/print-cfg' and exposes a graph representation of a file's CFG via external predicates ('selectedSourceFile', 'selected
Impact: The new 'printCfg.ql' query is tagged 'ide-contextual-queries/print-cfg' and exposes a graph representation of a file's CFG via external predicates ('selectedSourceFile', 'selectedSourceLine', 'selectedSourceColumn'). If this query is runnable in an environment where source file paths or line/column inputs are attacker-influenced, it could be used to exfiltrate code structure or probe internal source layout. The que…
Suggested fix: Review the cited evidence, fix the risk if confirmed, and rerun Shipwright.
1 participant
This is a very rough initial version, but it provides the basic functionality including "View CFG" support and consistency queries.
Source merge-base:
164d97da7456f78a78e8a1b8a27bacd18ab24604Source head:
f452bd48ebf0cf989ead1f81d769fba38b2f9a9d