Skip to content

Commit 496aab7

Browse files
authored
Merge pull request #8535 from aibaars/setter-method-arg-location
Ruby: fix location of setter-call argument
2 parents eff7cf6 + 06a99c3 commit 496aab7

File tree

8 files changed

+48
-40
lines changed

8 files changed

+48
-40
lines changed

ruby/ql/lib/codeql/ruby/ast/internal/Synthesis.qll

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -303,6 +303,14 @@ private module SetterDesugar {
303303
}
304304

305305
final override predicate location(AstNode n, Location l) {
306+
exists(SetterMethodCall sae, Assignment arg, AstNode lhs, AstNode rhs |
307+
arg = sae.getArgument(sae.getNumberOfArguments() - 1) and
308+
lhs = arg.getLeftOperand() and
309+
rhs = arg.getRightOperand() and
310+
n = [arg, lhs] and
311+
hasLocation(rhs, l)
312+
)
313+
or
306314
exists(SetterAssignExpr sae, StmtSequence seq |
307315
seq = sae.getDesugared() and
308316
l = sae.getMethodCallLocation() and

ruby/ql/test/library-tests/ast/AstDesugar.expected

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -65,18 +65,18 @@ calls/calls.rb:
6565
# 314| getStmt: [SetterMethodCall] call to foo=
6666
# 314| getReceiver: [SelfVariableAccess] self
6767
# 314| getArgument: [AssignExpr] ... = ...
68-
# 314| getAnOperand/getLeftOperand: [LocalVariableAccess] __synth__0
6968
# 314| getAnOperand/getRightOperand: [IntegerLiteral] 10
69+
# 314| getAnOperand/getLeftOperand: [LocalVariableAccess] __synth__0
7070
# 314| getStmt: [LocalVariableAccess] __synth__0
7171
# 315| [AssignExpr] ... = ...
7272
# 315| getDesugared: [StmtSequence] ...
7373
# 315| getStmt: [SetterMethodCall] call to []=
7474
# 315| getReceiver: [MethodCall] call to foo
7575
# 315| getReceiver: [SelfVariableAccess] self
76+
# 315| getArgument: [IntegerLiteral] 0
7677
# 315| getArgument: [AssignExpr] ... = ...
77-
# 315| getAnOperand/getLeftOperand: [LocalVariableAccess] __synth__0
7878
# 315| getAnOperand/getRightOperand: [IntegerLiteral] 10
79-
# 315| getArgument: [IntegerLiteral] 0
79+
# 315| getAnOperand/getLeftOperand: [LocalVariableAccess] __synth__0
8080
# 315| getStmt: [LocalVariableAccess] __synth__0
8181
# 316| [AssignExpr] ... = ...
8282
# 316| getDesugared: [StmtSequence] ...
@@ -857,76 +857,76 @@ gems/test.gemspec:
857857
# 2| getStmt: [SetterMethodCall] call to name=
858858
# 2| getReceiver: [LocalVariableAccess] s
859859
# 2| getArgument: [AssignExpr] ... = ...
860-
# 2| getAnOperand/getLeftOperand: [LocalVariableAccess] __synth__0
861860
# 2| getAnOperand/getRightOperand: [StringLiteral] "test"
862861
# 2| getComponent: [StringTextComponent] test
862+
# 2| getAnOperand/getLeftOperand: [LocalVariableAccess] __synth__0
863863
# 2| getStmt: [LocalVariableAccess] __synth__0
864864
# 3| [AssignExpr] ... = ...
865865
# 3| getDesugared: [StmtSequence] ...
866866
# 3| getStmt: [SetterMethodCall] call to version=
867867
# 3| getReceiver: [LocalVariableAccess] s
868868
# 3| getArgument: [AssignExpr] ... = ...
869-
# 3| getAnOperand/getLeftOperand: [LocalVariableAccess] __synth__0
870869
# 3| getAnOperand/getRightOperand: [StringLiteral] "0.0.0"
871870
# 3| getComponent: [StringTextComponent] 0.0.0
871+
# 3| getAnOperand/getLeftOperand: [LocalVariableAccess] __synth__0
872872
# 3| getStmt: [LocalVariableAccess] __synth__0
873873
# 4| [AssignExpr] ... = ...
874874
# 4| getDesugared: [StmtSequence] ...
875875
# 4| getStmt: [SetterMethodCall] call to summary=
876876
# 4| getReceiver: [LocalVariableAccess] s
877877
# 4| getArgument: [AssignExpr] ... = ...
878-
# 4| getAnOperand/getLeftOperand: [LocalVariableAccess] __synth__0
879878
# 4| getAnOperand/getRightOperand: [StringLiteral] "foo!"
880879
# 4| getComponent: [StringTextComponent] foo!
880+
# 4| getAnOperand/getLeftOperand: [LocalVariableAccess] __synth__0
881881
# 4| getStmt: [LocalVariableAccess] __synth__0
882882
# 5| [AssignExpr] ... = ...
883883
# 5| getDesugared: [StmtSequence] ...
884884
# 5| getStmt: [SetterMethodCall] call to description=
885885
# 5| getReceiver: [LocalVariableAccess] s
886886
# 5| getArgument: [AssignExpr] ... = ...
887-
# 5| getAnOperand/getLeftOperand: [LocalVariableAccess] __synth__0
888887
# 5| getAnOperand/getRightOperand: [StringLiteral] "A test"
889888
# 5| getComponent: [StringTextComponent] A test
889+
# 5| getAnOperand/getLeftOperand: [LocalVariableAccess] __synth__0
890890
# 5| getStmt: [LocalVariableAccess] __synth__0
891891
# 6| [AssignExpr] ... = ...
892892
# 6| getDesugared: [StmtSequence] ...
893893
# 6| getStmt: [SetterMethodCall] call to authors=
894894
# 6| getReceiver: [LocalVariableAccess] s
895895
# 6| getArgument: [AssignExpr] ... = ...
896-
# 6| getAnOperand/getLeftOperand: [LocalVariableAccess] __synth__0
897896
# 6| getAnOperand/getRightOperand: [ArrayLiteral] [...]
898897
# 6| getDesugared: [MethodCall] call to []
899898
# 6| getReceiver: [ConstantReadAccess] Array
900899
# 6| getArgument: [StringLiteral] "Mona Lisa"
901900
# 6| getComponent: [StringTextComponent] Mona Lisa
901+
# 6| getAnOperand/getLeftOperand: [LocalVariableAccess] __synth__0
902902
# 6| getStmt: [LocalVariableAccess] __synth__0
903903
# 7| [AssignExpr] ... = ...
904904
# 7| getDesugared: [StmtSequence] ...
905905
# 7| getStmt: [SetterMethodCall] call to email=
906906
# 7| getReceiver: [LocalVariableAccess] s
907907
# 7| getArgument: [AssignExpr] ... = ...
908-
# 7| getAnOperand/getLeftOperand: [LocalVariableAccess] __synth__0
909908
# 7| getAnOperand/getRightOperand: [StringLiteral] "mona@example.com"
910909
# 7| getComponent: [StringTextComponent] mona@example.com
910+
# 7| getAnOperand/getLeftOperand: [LocalVariableAccess] __synth__0
911911
# 7| getStmt: [LocalVariableAccess] __synth__0
912912
# 8| [AssignExpr] ... = ...
913913
# 8| getDesugared: [StmtSequence] ...
914914
# 8| getStmt: [SetterMethodCall] call to files=
915915
# 8| getReceiver: [LocalVariableAccess] s
916916
# 8| getArgument: [AssignExpr] ... = ...
917-
# 8| getAnOperand/getLeftOperand: [LocalVariableAccess] __synth__0
918917
# 8| getAnOperand/getRightOperand: [ArrayLiteral] [...]
919918
# 8| getDesugared: [MethodCall] call to []
920919
# 8| getReceiver: [ConstantReadAccess] Array
921920
# 8| getArgument: [StringLiteral] "lib/test.rb"
922921
# 8| getComponent: [StringTextComponent] lib/test.rb
922+
# 8| getAnOperand/getLeftOperand: [LocalVariableAccess] __synth__0
923923
# 8| getStmt: [LocalVariableAccess] __synth__0
924924
# 9| [AssignExpr] ... = ...
925925
# 9| getDesugared: [StmtSequence] ...
926926
# 9| getStmt: [SetterMethodCall] call to homepage=
927927
# 9| getReceiver: [LocalVariableAccess] s
928928
# 9| getArgument: [AssignExpr] ... = ...
929-
# 9| getAnOperand/getLeftOperand: [LocalVariableAccess] __synth__0
930929
# 9| getAnOperand/getRightOperand: [StringLiteral] "https://github.com/github/cod..."
931930
# 9| getComponent: [StringTextComponent] https://github.com/github/codeql-ruby
931+
# 9| getAnOperand/getLeftOperand: [LocalVariableAccess] __synth__0
932932
# 9| getStmt: [LocalVariableAccess] __synth__0

ruby/ql/test/library-tests/ast/calls/calls.expected

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -47,10 +47,10 @@ callsWithArguments
4747
| calls.rb:293:5:293:33 | call to super | super | 1 | calls.rb:293:14:293:14 | 7 |
4848
| calls.rb:293:23:293:29 | ... + ... | + | 0 | calls.rb:293:27:293:29 | 200 |
4949
| calls.rb:311:1:311:7 | call to call | call | 0 | calls.rb:311:6:311:6 | 1 |
50-
| calls.rb:314:1:314:8 | call to foo= | foo= | 0 | calls.rb:314:1:314:8 | ... = ... |
50+
| calls.rb:314:1:314:8 | call to foo= | foo= | 0 | calls.rb:314:12:314:13 | ... = ... |
5151
| calls.rb:315:1:315:6 | ...[...] | [] | 0 | calls.rb:315:5:315:5 | 0 |
5252
| calls.rb:315:1:315:6 | call to []= | []= | 0 | calls.rb:315:5:315:5 | 0 |
53-
| calls.rb:315:1:315:6 | call to []= | []= | 1 | calls.rb:315:1:315:6 | ... = ... |
53+
| calls.rb:315:1:315:6 | call to []= | []= | 1 | calls.rb:315:10:315:11 | ... = ... |
5454
| calls.rb:316:1:316:8 | call to [] | [] | 0 | calls.rb:316:1:316:8 | 0 |
5555
| calls.rb:316:1:316:8 | call to foo= | foo= | 0 | calls.rb:316:1:316:8 | ... = ... |
5656
| calls.rb:316:12:316:19 | call to [] | [] | 0 | calls.rb:316:12:316:19 | _ .. _ |

ruby/ql/test/library-tests/concepts/PersistentWriteAccess.expected

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,10 +13,10 @@
1313
| app/controllers/users_controller.rb:20:7:20:57 | call to update_attributes | app/controllers/users_controller.rb:20:37:20:41 | "U12" |
1414
| app/controllers/users_controller.rb:20:7:20:57 | call to update_attributes | app/controllers/users_controller.rb:20:49:20:55 | call to get_uid |
1515
| app/controllers/users_controller.rb:23:7:23:42 | call to update_attribute | app/controllers/users_controller.rb:23:37:23:41 | "U13" |
16-
| app/controllers/users_controller.rb:26:7:26:15 | ... = ... | app/controllers/users_controller.rb:26:19:26:23 | "U14" |
16+
| app/controllers/users_controller.rb:26:19:26:23 | ... = ... | app/controllers/users_controller.rb:26:19:26:23 | "U14" |
1717
| app/models/user.rb:4:5:4:28 | call to update | app/models/user.rb:4:23:4:27 | "U15" |
1818
| app/models/user.rb:5:5:5:23 | call to update | app/models/user.rb:5:18:5:22 | "U16" |
1919
| app/models/user.rb:6:5:6:56 | call to update_attributes | app/models/user.rb:6:35:6:39 | "U17" |
2020
| app/models/user.rb:6:5:6:56 | call to update_attributes | app/models/user.rb:6:51:6:54 | true |
2121
| app/models/user.rb:9:5:9:40 | call to update_attribute | app/models/user.rb:9:35:9:39 | "U18" |
22-
| app/models/user.rb:12:5:12:13 | ... = ... | app/models/user.rb:12:17:12:21 | "U19" |
22+
| app/models/user.rb:12:17:12:21 | ... = ... | app/models/user.rb:12:17:12:21 | "U19" |

ruby/ql/test/library-tests/controlflow/graph/Cfg.expected

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -3609,24 +3609,24 @@ desugar.rb:
36093609
# 6| call to foo
36103610
#-----| -> __synth__0
36113611

3612-
# 6| ... = ...
3613-
#-----| -> call to count=
3614-
36153612
# 6| __synth__0
36163613
#-----| -> ...
36173614

3618-
# 6| __synth__0
3619-
#-----| -> 1
3620-
36213615
# 6| call to count=
36223616
#-----| -> __synth__0
36233617

36243618
# 6| ...
36253619
#-----| -> exit m2 (normal)
36263620

3621+
# 6| ... = ...
3622+
#-----| -> call to count=
3623+
36273624
# 6| 1
36283625
#-----| -> ... = ...
36293626

3627+
# 6| __synth__0
3628+
#-----| -> 1
3629+
36303630
# 9| enter m3
36313631
#-----| -> x
36323632

@@ -3647,15 +3647,9 @@ desugar.rb:
36473647
# 10| call to foo
36483648
#-----| -> 0
36493649

3650-
# 10| ... = ...
3651-
#-----| -> call to []=
3652-
36533650
# 10| __synth__0
36543651
#-----| -> ...
36553652

3656-
# 10| __synth__0
3657-
#-----| -> 1
3658-
36593653
# 10| call to []=
36603654
#-----| -> __synth__0
36613655

@@ -3665,9 +3659,15 @@ desugar.rb:
36653659
# 10| 0
36663660
#-----| -> __synth__0
36673661

3662+
# 10| ... = ...
3663+
#-----| -> call to []=
3664+
36683665
# 10| 1
36693666
#-----| -> ... = ...
36703667

3668+
# 10| __synth__0
3669+
#-----| -> 1
3670+
36713671
# 13| enter m4
36723672
#-----| -> x
36733673

ruby/ql/test/library-tests/controlflow/graph/Nodes.expected

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -199,9 +199,9 @@ positionalArguments
199199
| cfg.rb:197:3:197:13 | call to bar | cfg.rb:197:7:197:7 | b |
200200
| cfg.rb:197:3:197:13 | call to bar | cfg.rb:197:10:197:12 | ... |
201201
| desugar.rb:2:5:2:6 | ... + ... | desugar.rb:2:8:2:8 | 1 |
202-
| desugar.rb:6:3:6:13 | call to count= | desugar.rb:6:3:6:13 | ... = ... |
203-
| desugar.rb:10:3:10:10 | call to []= | desugar.rb:10:3:10:10 | ... = ... |
202+
| desugar.rb:6:3:6:13 | call to count= | desugar.rb:6:17:6:17 | ... = ... |
204203
| desugar.rb:10:3:10:10 | call to []= | desugar.rb:10:9:10:9 | 0 |
204+
| desugar.rb:10:3:10:10 | call to []= | desugar.rb:10:14:10:14 | ... = ... |
205205
| desugar.rb:14:3:14:13 | call to count= | desugar.rb:14:15:14:16 | __synth__1 |
206206
| desugar.rb:14:15:14:16 | ... + ... | desugar.rb:14:18:14:18 | 1 |
207207
| desugar.rb:18:3:18:28 | call to [] | desugar.rb:18:9:18:9 | __synth__1 |

ruby/ql/test/library-tests/dataflow/type-tracker/TypeTracker.expected

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -50,9 +50,6 @@ track
5050
| type_tracker.rb:13:11:13:23 | call to new | type tracker without call steps | type_tracker.rb:13:11:13:23 | call to new |
5151
| type_tracker.rb:14:5:14:7 | [post] var | type tracker with call steps | type_tracker.rb:7:5:9:7 | self in field |
5252
| type_tracker.rb:14:5:14:7 | [post] var | type tracker without call steps | type_tracker.rb:14:5:14:7 | [post] var |
53-
| type_tracker.rb:14:5:14:13 | ... = ... | type tracker without call steps | type_tracker.rb:14:5:14:13 | ... = ... |
54-
| type_tracker.rb:14:5:14:13 | [post] ... = ... | type tracker without call steps | type_tracker.rb:14:5:14:13 | [post] ... = ... |
55-
| type_tracker.rb:14:5:14:13 | __synth__0 | type tracker without call steps | type_tracker.rb:14:5:14:13 | __synth__0 |
5653
| type_tracker.rb:14:5:14:13 | call to field= | type tracker without call steps | type_tracker.rb:14:5:14:13 | call to field= |
5754
| type_tracker.rb:14:17:14:23 | "hello" | type tracker with call steps | type_tracker.rb:2:16:2:18 | val |
5855
| type_tracker.rb:14:17:14:23 | "hello" | type tracker with call steps | type_tracker.rb:2:16:2:18 | val |
@@ -61,6 +58,9 @@ track
6158
| type_tracker.rb:14:17:14:23 | "hello" | type tracker without call steps | type_tracker.rb:14:17:14:23 | "hello" |
6259
| type_tracker.rb:14:17:14:23 | "hello" | type tracker without call steps | type_tracker.rb:15:10:15:18 | call to field |
6360
| type_tracker.rb:14:17:14:23 | "hello" | type tracker without call steps with content field | type_tracker.rb:14:5:14:7 | [post] var |
61+
| type_tracker.rb:14:17:14:23 | ... = ... | type tracker without call steps | type_tracker.rb:14:17:14:23 | ... = ... |
62+
| type_tracker.rb:14:17:14:23 | [post] ... = ... | type tracker without call steps | type_tracker.rb:14:17:14:23 | [post] ... = ... |
63+
| type_tracker.rb:14:17:14:23 | __synth__0 | type tracker without call steps | type_tracker.rb:14:17:14:23 | __synth__0 |
6464
| type_tracker.rb:15:5:15:18 | [post] self | type tracker without call steps | type_tracker.rb:15:5:15:18 | [post] self |
6565
| type_tracker.rb:15:5:15:18 | call to puts | type tracker without call steps | type_tracker.rb:12:1:16:3 | return return in m |
6666
| type_tracker.rb:15:5:15:18 | call to puts | type tracker without call steps | type_tracker.rb:15:5:15:18 | call to puts |
@@ -142,23 +142,23 @@ trackEnd
142142
| type_tracker.rb:14:5:14:7 | [post] var | type_tracker.rb:7:5:9:7 | self in field |
143143
| type_tracker.rb:14:5:14:7 | [post] var | type_tracker.rb:14:5:14:7 | [post] var |
144144
| type_tracker.rb:14:5:14:7 | [post] var | type_tracker.rb:15:10:15:12 | var |
145-
| type_tracker.rb:14:5:14:13 | ... = ... | type_tracker.rb:14:5:14:13 | ... = ... |
146-
| type_tracker.rb:14:5:14:13 | ... = ... | type_tracker.rb:14:5:14:13 | __synth__0 |
147-
| type_tracker.rb:14:5:14:13 | ... = ... | type_tracker.rb:14:5:14:23 | ... |
148-
| type_tracker.rb:14:5:14:13 | [post] ... = ... | type_tracker.rb:14:5:14:13 | [post] ... = ... |
149-
| type_tracker.rb:14:5:14:13 | __synth__0 | type_tracker.rb:14:5:14:13 | __synth__0 |
150145
| type_tracker.rb:14:5:14:13 | call to field= | type_tracker.rb:14:5:14:13 | call to field= |
151146
| type_tracker.rb:14:17:14:23 | "hello" | type_tracker.rb:2:16:2:18 | val |
152147
| type_tracker.rb:14:17:14:23 | "hello" | type_tracker.rb:2:16:2:18 | val |
153148
| type_tracker.rb:14:17:14:23 | "hello" | type_tracker.rb:4:9:4:20 | ... = ... |
154149
| type_tracker.rb:14:17:14:23 | "hello" | type_tracker.rb:4:18:4:20 | val |
155-
| type_tracker.rb:14:17:14:23 | "hello" | type_tracker.rb:14:5:14:13 | ... = ... |
156-
| type_tracker.rb:14:17:14:23 | "hello" | type_tracker.rb:14:5:14:13 | ... = ... |
157150
| type_tracker.rb:14:17:14:23 | "hello" | type_tracker.rb:14:5:14:13 | __synth__0 |
158151
| type_tracker.rb:14:17:14:23 | "hello" | type_tracker.rb:14:5:14:13 | call to field= |
159152
| type_tracker.rb:14:17:14:23 | "hello" | type_tracker.rb:14:5:14:23 | ... |
160153
| type_tracker.rb:14:17:14:23 | "hello" | type_tracker.rb:14:17:14:23 | "hello" |
154+
| type_tracker.rb:14:17:14:23 | "hello" | type_tracker.rb:14:17:14:23 | ... = ... |
155+
| type_tracker.rb:14:17:14:23 | "hello" | type_tracker.rb:14:17:14:23 | ... = ... |
161156
| type_tracker.rb:14:17:14:23 | "hello" | type_tracker.rb:15:10:15:18 | call to field |
157+
| type_tracker.rb:14:17:14:23 | ... = ... | type_tracker.rb:14:5:14:13 | __synth__0 |
158+
| type_tracker.rb:14:17:14:23 | ... = ... | type_tracker.rb:14:5:14:23 | ... |
159+
| type_tracker.rb:14:17:14:23 | ... = ... | type_tracker.rb:14:17:14:23 | ... = ... |
160+
| type_tracker.rb:14:17:14:23 | [post] ... = ... | type_tracker.rb:14:17:14:23 | [post] ... = ... |
161+
| type_tracker.rb:14:17:14:23 | __synth__0 | type_tracker.rb:14:17:14:23 | __synth__0 |
162162
| type_tracker.rb:15:5:15:18 | [post] self | type_tracker.rb:15:5:15:18 | [post] self |
163163
| type_tracker.rb:15:5:15:18 | call to puts | type_tracker.rb:12:1:16:3 | return return in m |
164164
| type_tracker.rb:15:5:15:18 | call to puts | type_tracker.rb:15:5:15:18 | call to puts |

ruby/ql/test/query-tests/security/cwe-295/RequestWithoutValidation.expected

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@
1111
| Httparty.rb:10:1:10:59 | call to get | This request may run with $@. | Httparty.rb:10:39:10:56 | Pair | certificate validation disabled |
1212
| Httparty.rb:13:1:13:70 | call to post | This request may run with $@. | Httparty.rb:13:57:13:69 | Pair | certificate validation disabled |
1313
| Httparty.rb:16:1:16:74 | call to post | This request may run with $@. | Httparty.rb:16:59:16:71 | Pair | certificate validation disabled |
14-
| NetHttp.rb:9:12:9:31 | call to request | This request may run with $@. | NetHttp.rb:7:1:7:16 | ... = ... | certificate validation disabled |
14+
| NetHttp.rb:9:12:9:31 | call to request | This request may run with $@. | NetHttp.rb:7:20:7:44 | ... = ... | certificate validation disabled |
1515
| OpenURI.rb:4:1:4:78 | call to open | This request may run with $@. | OpenURI.rb:4:36:4:77 | Pair | certificate validation disabled |
1616
| OpenURI.rb:7:1:7:82 | call to open | This request may run with $@. | OpenURI.rb:7:38:7:79 | Pair | certificate validation disabled |
1717
| OpenURI.rb:11:1:11:43 | call to open | This request may run with $@. | OpenURI.rb:10:13:10:54 | Pair | certificate validation disabled |

0 commit comments

Comments
 (0)