Skip to content

Commit dc0cb48

Browse files
owen-mcowen-mc
committed
WIP
1 parent 19b8a08 commit dc0cb48

16 files changed

Lines changed: 5806 additions & 25 deletions

File tree

‎categories.md‎

Lines changed: 214 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,214 @@
1+
Updated todo list
2+
3+
Read [](file:///Users/owen-mc/workspace/code/ql/temp#1-1), lines 1 to 100
4+
5+
Read [](file:///Users/owen-mc/workspace/code/ql/temp#101-101), lines 101 to 250
6+
7+
Read [](file:///Users/owen-mc/workspace/code/ql/temp#251-251), lines 251 to 400
8+
9+
Read [](file:///Users/owen-mc/workspace/code/ql/temp#401-401), lines 401 to 600
10+
11+
Read [](file:///Users/owen-mc/workspace/code/ql/temp#601-601), lines 601 to 800
12+
13+
Read [](file:///Users/owen-mc/workspace/code/ql/temp#801-801), lines 801 to 1000
14+
15+
Read [](file:///Users/owen-mc/workspace/code/ql/temp#1001-1001), lines 1001 to 1200
16+
17+
Read [](file:///Users/owen-mc/workspace/code/ql/temp#1201-1201), lines 1201 to 1400
18+
19+
Read [](file:///Users/owen-mc/workspace/code/ql/temp#1401-1401), lines 1401 to 1600
20+
21+
Ran terminal command: wc -l /Users/owen-mc/workspace/code/ql/temp
22+
23+
Ran terminal command: grep "^diff --git" /Users/owen-mc/workspace/code/ql/temp | wc -l
24+
25+
Ran terminal command: grep "^diff --git" /Users/owen-mc/workspace/code/ql/temp | grep -v "CONSISTENCY\|Consistency" | head -20
26+
27+
Ran terminal command: grep "^diff --git" /Users/owen-mc/workspace/code/ql/temp | grep "CONSISTENCY\|Consistency" | sed 's/.*CONSISTENCY\///' | sort | uniq -c | sort -rn
28+
29+
Ran terminal command: grep "^diff --git" /Users/owen-mc/workspace/code/ql/temp | grep "CfgConsistency" | head -5
30+
31+
Now let me look at the types of CfgConsistency issues more carefully. Let me check if they are new files vs modifications:
32+
33+
Ran terminal command: grep -B1 "^new file" /Users/owen-mc/workspace/code/ql/temp | grep "^diff" | grep "CfgConsistency" | wc -l
34+
35+
Ran terminal command: grep -B1 "^new file" /Users/owen-mc/workspace/code/ql/temp | grep "^diff" | grep "DataFlowConsistency" | wc -l
36+
37+
So all 91 CfgConsistency changes are new files and all 46 DataFlowConsistency changes are modifications to existing files. Let me now analyze the types of CfgConsistency issues:
38+
39+
Ran terminal command: grep "^\+|" /Users/owen-mc/workspace/code/ql/temp | grep -v "consistencyOverview" | awk -F'|' '{print $2}' | sort | uniq -c | sort -rn | head -20
40+
41+
Now let me look at the specific patterns within the CfgConsistency new files. The common patterns seem to be:
42+
1. Block statements with self-loops and multiple successors (empty function bodies)
43+
2. Return statements with multiple successors (return to Normal Exit AND After block statement)
44+
3. Missing in-node for post or in order (parenthesized expressions)
45+
4. Select statements with multiple successors
46+
5. Dead ends (select statements)
47+
48+
Let me also check the DataFlowConsistency changes:
49+
50+
Ran terminal command: grep "^-|" /Users/owen-mc/workspace/code/ql/temp | grep "DataFlowConsistency" 2>/dev/null; grep "^-|" /Users/owen-mc/workspace/code/ql/temp | sort -u | head -10
51+
52+
Ran terminal command: grep "^-|" /Users/owen-mc/workspace/code/ql/temp | grep "implicit dereference" | wc -l
53+
54+
Ran terminal command: grep "^\+|" /Users/owen-mc/workspace/code/ql/temp | grep "implicit-deref" | wc -l
55+
56+
Ran terminal command: grep "^-|" /Users/owen-mc/workspace/code/ql/temp | grep -v "implicit dereference" | head -20
57+
58+
Ran terminal command: grep "^-|" /Users/owen-mc/workspace/code/ql/temp | grep -v "implicit dereference" | wc -l
59+
60+
Ran terminal command: grep "^-|" /Users/owen-mc/workspace/code/ql/temp | grep "implicit read of field" | wc -l
61+
62+
Ran terminal command: grep "^\+|" /Users/owen-mc/workspace/code/ql/temp | grep "implicit read of field\|implicit-read" | wc -l
63+
64+
So the "implicit read of field" errors were removed (fixed). Let me check what replaced them or if they're just gone:
65+
66+
Ran terminal command: grep -A5 "implicit read of field Controller" /Users/owen-mc/workspace/code/ql/temp | grep "^\+" | head -5
67+
68+
Ran terminal command: grep "^\+|" /Users/owen-mc/workspace/code/ql/temp | grep "implicit-field" | wc -l
69+
70+
So "implicit read of field Controller" → "implicit-field:N selection of Params" (renamed, 55 lines). Let me now look at the remaining removals:
71+
72+
Ran terminal command: grep "^-|" /Users/owen-mc/workspace/code/ql/temp | grep -v "implicit dereference\|implicit read of field" | sort -u
73+
74+
Now let me look at the remaining portions of the file to see the other non-consistency changes and the Revel/Twirp framework tests:
75+
76+
Read [](file:///Users/owen-mc/workspace/code/ql/temp#1601-1601), lines 1601 to 1800
77+
78+
Read [](file:///Users/owen-mc/workspace/code/ql/temp#1801-1801), lines 1801 to 2000
79+
80+
Let me continue reading the remaining key sections:
81+
82+
Ran terminal command: grep "^diff --git" /Users/owen-mc/workspace/code/ql/temp | tail -30
83+
84+
Now let me check the CleartextLogging.expected and the other non-consistency-test changes:
85+
86+
Ran terminal command: grep -A50 "CleartextLogging.expected" /Users/owen-mc/workspace/code/ql/temp | head -80
87+
88+
Read [](file:///Users/owen-mc/Library/Application%20Support/Code%20-%20Insiders/User/workspaceStorage/507184b1f763e11bd182e0cf565259f0/GitHub.copilot-chat/chat-session-resources/e68ac4b7-2781-4cd4-a8cc-2487136525a5/toolu_bdrk_01Vho2XXhdRu9D8ikNuHMYog__vscode-1782728597633/content.txt#1-1), lines 1 to 80
89+
90+
Now let me also check the Twirp/RequestForgery expected change:
91+
92+
Ran terminal command: grep -A20 "Twirp/RequestForgery" /Users/owen-mc/workspace/code/ql/temp | head -30
93+
94+
Now let me check what other non-consistency changes exist (the VarArgs DataFlowConsistency removed result and the ControlFlowGraph/NoretFunctions):
95+
96+
Ran terminal command: grep -A10 "VarArgs/CONSISTENCY/DataFlowConsistency" /Users/owen-mc/workspace/code/ql/temp
97+
98+
Now let me get a final count of the CfgConsistency patterns to properly categorize them:
99+
100+
Ran terminal command: grep -c "block statement.*selfLoop\|block statement.*successor.*block statement" /Users/owen-mc/workspace/code/ql/temp
101+
102+
Ran terminal command: grep "^\+|.*return statement.*return.*Normal Exit" /Users/owen-mc/workspace/code/ql/temp | wc -l
103+
104+
Ran terminal command: grep "^\+|.*return statement.*return.*After block statement" /Users/owen-mc/workspace/code/ql/temp | wc -l
105+
106+
Ran terminal command: grep "^\+|.*missingInNodeForPostOrInOrder" /Users/owen-mc/workspace/code/ql/temp | wc -l
107+
108+
Ran terminal command: grep "^\+|.*(\.\.\.)" /Users/owen-mc/workspace/code/ql/temp | wc -l
109+
110+
Ran terminal command: grep "^\+|.*deadEnd\|^\+|.*Dead" /Users/owen-mc/workspace/code/ql/temp | wc -l
111+
112+
Ran terminal command: grep "^\+|.*select statement" /Users/owen-mc/workspace/code/ql/temp | wc -l
113+
114+
Ran terminal command: grep "^\+|.*defer-invoke" /Users/owen-mc/workspace/code/ql/temp | wc -l
115+
116+
Now I have enough information to provide a comprehensive summary. Here are the consistency test changes grouped by cause:
117+
118+
---
119+
120+
## Consistency Test Changes Grouped by Cause
121+
122+
### 1. Implicit dereference node renaming (447 lines across 46 DataFlowConsistency files)
123+
**Pattern:** `implicit dereference` → `implicit-deref <varname>`
124+
125+
All existing DataFlowConsistency reverseRead errors remain but with a renamed label. Previously nodes were generically called `"implicit dereference"`, now they include the variable name (e.g., `implicit-deref req`, `implicit-deref requestCtx`, `implicit-deref c`). This is purely a cosmetic/labeling change — same inconsistency, better name.
126+
127+
**Files:** All 46 modified DataFlowConsistency.expected files.
128+
129+
---
130+
131+
### 2. Implicit field read node renaming (55 lines, primarily Revel framework)
132+
**Pattern:** `implicit read of field Controller` → `implicit-field:1 selection of <FieldName>`
133+
134+
Previously, reads through embedded struct fields were labeled `"implicit read of field Controller"` (always showing the embedding field). Now they show the actual accessed field like `"implicit-field:1 selection of Params"`, `"implicit-field:1 selection of Request"`, `"implicit-field:1 selection of Session"`, etc.
135+
136+
**Files:** Primarily `Revel/CONSISTENCY/DataFlowConsistency.expected`.
137+
138+
---
139+
140+
### 3. Empty function body CFG inconsistencies — new (91 new CfgConsistency files)
141+
**Pattern:** Block statement has multiple successors (itself + Normal Exit) and a self-loop.
142+
143+
These are brand-new CfgConsistency.expected files that were previously empty. They represent functions with empty bodies where the block statement node has both a successor to itself (selfLoop) and to Normal Exit (multipleSuccessors). Appears across ~60+ test directories.
144+
145+
**Example:**
146+
```
147+
| main.go:5:13:5:14 | block statement | successor | main.go:5:1:5:14 | Normal Exit |
148+
| main.go:5:13:5:14 | block statement | successor | main.go:5:13:5:14 | block statement |
149+
```
150+
151+
---
152+
153+
### 4. Return statement has multiple successors — new (232 pairs in new CfgConsistency files)
154+
**Pattern:** Return statement goes to both `Normal Exit` and `After block statement`.
155+
156+
Functions with named return values where a `return` statement inside a nested block has two successors — the function's Normal Exit and an "After block statement" node.
157+
158+
**Example:**
159+
```
160+
| test.go:41:3:41:13 | return statement | return | test.go:38:1:49:1 | Normal Exit |
161+
| test.go:41:3:41:13 | return statement | return | test.go:38:66:49:1 | After block statement |
162+
```
163+
164+
---
165+
166+
### 5. `missingInNodeForPostOrInOrder` — parenthesized expressions (52 entries in new CfgConsistency files)
167+
**Pattern:** `(...) | (...) |` entries for parenthesized type conversion expressions.
168+
169+
These are parenthesized expressions (type conversions) that lack proper in-order CFG nodes.
170+
171+
---
172+
173+
### 6. Select statements with multiple successors and dead ends (new CfgConsistency files)
174+
**Pattern:** Select statements have multiple comm clause successors, and select statements without a default case are dead ends.
175+
176+
---
177+
178+
### 7. Defer-invoke changes in ControlFlowGraph tests (ControlFlowNode_getASuccessor.expected, NoretFunctions.expected)
179+
**Pattern:** Functions calling `panic` with deferred `recover` are no longer considered no-return.
180+
181+
`epiRecoverUnnamed` and `canRecover` were **removed** from `NoretFunctions.expected` because these functions defer a recover call, meaning they *can* return normally. New CFG edges were added showing `panic → defer-invoke call → Normal Exit`.
182+
183+
---
184+
185+
### 8. Extract node changes in IR and CFG tests
186+
**Pattern:** New `extract:0` / `extract:1` nodes for multi-return function calls.
187+
188+
In `IR/test.expected`, new extract nodes appear for calls to multi-return functions. In `ControlFlowNode_getASuccessor.expected`, the call to `add(gen())` now routes through extract nodes before the outer call.
189+
190+
---
191+
192+
### 9. GlobalValueNumbering changes
193+
**Pattern:** New `result-zero-init:0` GVN entries and changed SSA defs for named return variables.
194+
195+
Named return variables now have zero-init GVN entries, and the result-read values have shifted due to accounting for assignments at different points.
196+
197+
---
198+
199+
### 10. CleartextLogging query result removals (CleartextLogging.expected)
200+
**Pattern:** Removed false-positive alerts for key-value pairs in struct literals.
201+
202+
3 results removed where flow through struct literal key-value pairs was being reported as a cleartext logging vulnerability (passwords.go lines 39, 65, 89, 123). These appear to be false positives that were eliminated.
203+
204+
---
205+
206+
### 11. VarArgs DataFlowConsistency fix
207+
**Pattern:** Removed `out | Origin of readStep is missing a PostUpdateNode` — a genuine fix.
208+
209+
---
210+
211+
### 12. Twirp comment column shift
212+
**Pattern:** `19:111:19:154 | comment` → `19:111:19:146 | comment` — minor column change in source location.
213+
214+
Created 3 todos

‎go/ql/test/experimental/InconsistentCode/CONSISTENCY/CfgConsistency.expected‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
consistencyOverview
2+
| multipleSuccessors | 4 |
3+
| selfLoop | 1 |
4+
multipleSuccessors
5+
| DeferInLoop.go:5:36:14:1 | After block statement | successor | DeferInLoop.go:5:1:14:1 | Normal Exit |
6+
| DeferInLoop.go:5:36:14:1 | After block statement | successor | DeferInLoop.go:8:9:8:20 | defer-invoke call to Close |
7+
| DeferInLoop.go:8:9:8:20 | defer-invoke call to Close | successor | DeferInLoop.go:5:1:14:1 | Normal Exit |
8+
| DeferInLoop.go:8:9:8:20 | defer-invoke call to Close | successor | DeferInLoop.go:8:9:8:20 | defer-invoke call to Close |
9+
selfLoop
10+
| DeferInLoop.go:8:9:8:20 | defer-invoke call to Close | successor |

‎go/ql/test/library-tests/semmle/go/IR/test.expected‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,3 +4,7 @@
44
| test.go:15:2:15:20 | extract:1 ... := ... | test.go:15:13:15:20 | index expression | 1 | file://:0:0:0:0 | bool |
55
| test.go:21:2:21:22 | extract:0 ... := ... | test.go:21:13:21:22 | type assertion | 0 | file://:0:0:0:0 | string |
66
| test.go:21:2:21:22 | extract:1 ... := ... | test.go:21:13:21:22 | type assertion | 1 | file://:0:0:0:0 | bool |
7+
| test.go:29:2:29:7 | extract:0 call to f | test.go:29:4:29:6 | call to g | 0 | file://:0:0:0:0 | int |
8+
| test.go:29:2:29:7 | extract:1 call to f | test.go:29:4:29:6 | call to g | 1 | file://:0:0:0:0 | int |
9+
| test.go:33:2:33:7 | extract:0 call to f | test.go:33:4:33:6 | call to v | 0 | file://:0:0:0:0 | int |
10+
| test.go:33:2:33:7 | extract:1 call to f | test.go:33:4:33:6 | call to v | 1 | file://:0:0:0:0 | int |

‎go/ql/test/library-tests/semmle/go/PrintAst/CONSISTENCY/CfgConsistency.expected‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
consistencyOverview
2+
| deadEnd | 1 |
3+
| multipleSuccessors | 6 |
4+
deadEnd
5+
| input.go:61:2:61:10 | select statement |
6+
multipleSuccessors
7+
| input.go:50:2:59:2 | select statement | successor | input.go:51:2:52:31 | comm clause |
8+
| input.go:50:2:59:2 | select statement | successor | input.go:53:2:55:16 | comm clause |
9+
| input.go:50:2:59:2 | select statement | successor | input.go:56:2:57:15 | comm clause |
10+
| input.go:50:2:59:2 | select statement | successor | input.go:58:2:58:16 | comm clause |
11+
| input.go:71:2:71:10 | return statement | successor | input.go:67:9:67:35 | defer-invoke function call |
12+
| input.go:71:2:71:10 | return statement | successor | input.go:69:9:69:36 | defer-invoke function call |

‎go/ql/test/library-tests/semmle/go/controlflow/ControlFlowGraph/CONSISTENCY/CfgConsistency.expected‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
consistencyOverview
2+
| deadEnd | 1 |
3+
| multipleSuccessors | 10 |
4+
deadEnd
5+
| stmts.go:61:2:61:10 | select statement |
6+
multipleSuccessors
7+
| stmts2.go:16:2:26:2 | select statement | successor | stmts2.go:17:2:17:15 | comm clause |
8+
| stmts2.go:16:2:26:2 | select statement | successor | stmts2.go:18:2:19:10 | comm clause |
9+
| stmts2.go:16:2:26:2 | select statement | successor | stmts2.go:20:2:24:10 | comm clause |
10+
| stmts2.go:16:2:26:2 | select statement | successor | stmts2.go:25:2:25:18 | comm clause |
11+
| stmts.go:50:2:59:2 | select statement | successor | stmts.go:51:2:52:31 | comm clause |
12+
| stmts.go:50:2:59:2 | select statement | successor | stmts.go:53:2:55:16 | comm clause |
13+
| stmts.go:50:2:59:2 | select statement | successor | stmts.go:56:2:57:15 | comm clause |
14+
| stmts.go:50:2:59:2 | select statement | successor | stmts.go:58:2:58:16 | comm clause |
15+
| stmts.go:71:2:71:10 | return statement | successor | stmts.go:67:9:67:35 | defer-invoke function call |
16+
| stmts.go:71:2:71:10 | return statement | successor | stmts.go:69:9:69:36 | defer-invoke function call |

0 commit comments

Comments
 (0)