Ruby: fix location of setter-call argument

2026-05-01 19:55:15 +02:00 · 2022-03-23 11:41:19 +01:00
parent efc0d95535
commit 06a99c3987
8 changed files with 49 additions and 41 deletions
--- a/ruby/ql/lib/codeql/ruby/ast/internal/Synthesis.qll
+++ b/ruby/ql/lib/codeql/ruby/ast/internal/Synthesis.qll
@@ -303,6 +303,14 @@ private module SetterDesugar {
    }

    final override predicate location(AstNode n, Location l) {
+      exists(SetterMethodCall sae, Assignment arg, AstNode lhs, AstNode rhs |
+        arg = sae.getArgument(sae.getNumberOfArguments() - 1) and
+        lhs = arg.getLeftOperand() and
+        rhs = arg.getRightOperand() and
+        n = [arg, lhs] and
+        hasLocation(rhs, l)
+      )
+      or
      exists(SetterAssignExpr sae, StmtSequence seq |
        seq = sae.getDesugared() and
        l = sae.getMethodCallLocation() and
--- a/ruby/ql/test/library-tests/ast/AstDesugar.expected
+++ b/ruby/ql/test/library-tests/ast/AstDesugar.expected
@@ -65,18 +65,18 @@ calls/calls.rb:
 #  314|     getStmt: [SetterMethodCall] call to foo=
 #  314|       getReceiver: [SelfVariableAccess] self
 #  314|       getArgument: [AssignExpr] ... = ...
-#  314|         getAnOperand/getLeftOperand: [LocalVariableAccess] __synth__0
 #  314|         getAnOperand/getRightOperand: [IntegerLiteral] 10
+#  314|         getAnOperand/getLeftOperand: [LocalVariableAccess] __synth__0
 #  314|     getStmt: [LocalVariableAccess] __synth__0
 #  315| [AssignExpr] ... = ...
 #  315|   getDesugared: [StmtSequence] ...
 #  315|     getStmt: [SetterMethodCall] call to []=
 #  315|       getReceiver: [MethodCall] call to foo
 #  315|         getReceiver: [SelfVariableAccess] self
-#  315|       getArgument: [AssignExpr] ... = ...
-#  315|         getAnOperand/getLeftOperand: [LocalVariableAccess] __synth__0
-#  315|         getAnOperand/getRightOperand: [IntegerLiteral] 10
 #  315|       getArgument: [IntegerLiteral] 0
+#  315|       getArgument: [AssignExpr] ... = ...
+#  315|         getAnOperand/getRightOperand: [IntegerLiteral] 10
+#  315|         getAnOperand/getLeftOperand: [LocalVariableAccess] __synth__0
 #  315|     getStmt: [LocalVariableAccess] __synth__0
 #  316| [AssignExpr] ... = ...
 #  316|   getDesugared: [StmtSequence] ...
@@ -857,76 +857,76 @@ gems/test.gemspec:
 #    2|     getStmt: [SetterMethodCall] call to name=
 #    2|       getReceiver: [LocalVariableAccess] s
 #    2|       getArgument: [AssignExpr] ... = ...
-#    2|         getAnOperand/getLeftOperand: [LocalVariableAccess] __synth__0
 #    2|         getAnOperand/getRightOperand: [StringLiteral] "test"
 #    2|           getComponent: [StringTextComponent] test
+#    2|         getAnOperand/getLeftOperand: [LocalVariableAccess] __synth__0
 #    2|     getStmt: [LocalVariableAccess] __synth__0
 #    3| [AssignExpr] ... = ...
 #    3|   getDesugared: [StmtSequence] ...
 #    3|     getStmt: [SetterMethodCall] call to version=
 #    3|       getReceiver: [LocalVariableAccess] s
 #    3|       getArgument: [AssignExpr] ... = ...
-#    3|         getAnOperand/getLeftOperand: [LocalVariableAccess] __synth__0
 #    3|         getAnOperand/getRightOperand: [StringLiteral] "0.0.0"
 #    3|           getComponent: [StringTextComponent] 0.0.0
+#    3|         getAnOperand/getLeftOperand: [LocalVariableAccess] __synth__0
 #    3|     getStmt: [LocalVariableAccess] __synth__0
 #    4| [AssignExpr] ... = ...
 #    4|   getDesugared: [StmtSequence] ...
 #    4|     getStmt: [SetterMethodCall] call to summary=
 #    4|       getReceiver: [LocalVariableAccess] s
 #    4|       getArgument: [AssignExpr] ... = ...
-#    4|         getAnOperand/getLeftOperand: [LocalVariableAccess] __synth__0
 #    4|         getAnOperand/getRightOperand: [StringLiteral] "foo!"
 #    4|           getComponent: [StringTextComponent] foo!
+#    4|         getAnOperand/getLeftOperand: [LocalVariableAccess] __synth__0
 #    4|     getStmt: [LocalVariableAccess] __synth__0
 #    5| [AssignExpr] ... = ...
 #    5|   getDesugared: [StmtSequence] ...
 #    5|     getStmt: [SetterMethodCall] call to description=
 #    5|       getReceiver: [LocalVariableAccess] s
 #    5|       getArgument: [AssignExpr] ... = ...
-#    5|         getAnOperand/getLeftOperand: [LocalVariableAccess] __synth__0
 #    5|         getAnOperand/getRightOperand: [StringLiteral] "A test"
 #    5|           getComponent: [StringTextComponent] A test
+#    5|         getAnOperand/getLeftOperand: [LocalVariableAccess] __synth__0
 #    5|     getStmt: [LocalVariableAccess] __synth__0
 #    6| [AssignExpr] ... = ...
 #    6|   getDesugared: [StmtSequence] ...
 #    6|     getStmt: [SetterMethodCall] call to authors=
 #    6|       getReceiver: [LocalVariableAccess] s
 #    6|       getArgument: [AssignExpr] ... = ...
-#    6|         getAnOperand/getLeftOperand: [LocalVariableAccess] __synth__0
 #    6|         getAnOperand/getRightOperand: [ArrayLiteral] [...]
 #    6|           getDesugared: [MethodCall] call to []
 #    6|             getReceiver: [ConstantReadAccess] Array
 #    6|             getArgument: [StringLiteral] "Mona Lisa"
 #    6|               getComponent: [StringTextComponent] Mona Lisa
+#    6|         getAnOperand/getLeftOperand: [LocalVariableAccess] __synth__0
 #    6|     getStmt: [LocalVariableAccess] __synth__0
 #    7| [AssignExpr] ... = ...
 #    7|   getDesugared: [StmtSequence] ...
 #    7|     getStmt: [SetterMethodCall] call to email=
 #    7|       getReceiver: [LocalVariableAccess] s
 #    7|       getArgument: [AssignExpr] ... = ...
-#    7|         getAnOperand/getLeftOperand: [LocalVariableAccess] __synth__0
 #    7|         getAnOperand/getRightOperand: [StringLiteral] "mona@example.com"
 #    7|           getComponent: [StringTextComponent] mona@example.com
+#    7|         getAnOperand/getLeftOperand: [LocalVariableAccess] __synth__0
 #    7|     getStmt: [LocalVariableAccess] __synth__0
 #    8| [AssignExpr] ... = ...
 #    8|   getDesugared: [StmtSequence] ...
 #    8|     getStmt: [SetterMethodCall] call to files=
 #    8|       getReceiver: [LocalVariableAccess] s
 #    8|       getArgument: [AssignExpr] ... = ...
-#    8|         getAnOperand/getLeftOperand: [LocalVariableAccess] __synth__0
 #    8|         getAnOperand/getRightOperand: [ArrayLiteral] [...]
 #    8|           getDesugared: [MethodCall] call to []
 #    8|             getReceiver: [ConstantReadAccess] Array
 #    8|             getArgument: [StringLiteral] "lib/test.rb"
 #    8|               getComponent: [StringTextComponent] lib/test.rb
+#    8|         getAnOperand/getLeftOperand: [LocalVariableAccess] __synth__0
 #    8|     getStmt: [LocalVariableAccess] __synth__0
 #    9| [AssignExpr] ... = ...
 #    9|   getDesugared: [StmtSequence] ...
 #    9|     getStmt: [SetterMethodCall] call to homepage=
 #    9|       getReceiver: [LocalVariableAccess] s
 #    9|       getArgument: [AssignExpr] ... = ...
-#    9|         getAnOperand/getLeftOperand: [LocalVariableAccess] __synth__0
 #    9|         getAnOperand/getRightOperand: [StringLiteral] "https://github.com/github/cod..."
 #    9|           getComponent: [StringTextComponent] https://github.com/github/codeql-ruby
+#    9|         getAnOperand/getLeftOperand: [LocalVariableAccess] __synth__0
 #    9|     getStmt: [LocalVariableAccess] __synth__0
--- a/ruby/ql/test/library-tests/ast/calls/calls.expected
+++ b/ruby/ql/test/library-tests/ast/calls/calls.expected
@@ -47,10 +47,10 @@ callsWithArguments
 | calls.rb:293:5:293:33 | call to super | super | 1 | calls.rb:293:14:293:14 | 7 |
 | calls.rb:293:23:293:29 | ... + ... | + | 0 | calls.rb:293:27:293:29 | 200 |
 | calls.rb:311:1:311:7 | call to call | call | 0 | calls.rb:311:6:311:6 | 1 |
-| calls.rb:314:1:314:8 | call to foo= | foo= | 0 | calls.rb:314:1:314:8 | ... = ... |
+| calls.rb:314:1:314:8 | call to foo= | foo= | 0 | calls.rb:314:12:314:13 | ... = ... |
 | calls.rb:315:1:315:6 | ...[...] | [] | 0 | calls.rb:315:5:315:5 | 0 |
 | calls.rb:315:1:315:6 | call to []= | []= | 0 | calls.rb:315:5:315:5 | 0 |
-| calls.rb:315:1:315:6 | call to []= | []= | 1 | calls.rb:315:1:315:6 | ... = ... |
+| calls.rb:315:1:315:6 | call to []= | []= | 1 | calls.rb:315:10:315:11 | ... = ... |
 | calls.rb:316:1:316:8 | call to [] | [] | 0 | calls.rb:316:1:316:8 | 0 |
 | calls.rb:316:1:316:8 | call to foo= | foo= | 0 | calls.rb:316:1:316:8 | ... = ... |
 | calls.rb:316:12:316:19 | call to [] | [] | 0 | calls.rb:316:12:316:19 | _ .. _ |
--- a/ruby/ql/test/library-tests/concepts/PersistentWriteAccess.expected
+++ b/ruby/ql/test/library-tests/concepts/PersistentWriteAccess.expected
@@ -13,10 +13,10 @@
 | app/controllers/users_controller.rb:20:7:20:57 | call to update_attributes | app/controllers/users_controller.rb:20:37:20:41 | "U12" |
 | 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 |
 | app/controllers/users_controller.rb:23:7:23:42 | call to update_attribute | app/controllers/users_controller.rb:23:37:23:41 | "U13" |
-| app/controllers/users_controller.rb:26:7:26:15 | ... = ... | app/controllers/users_controller.rb:26:19:26:23 | "U14" |
+| app/controllers/users_controller.rb:26:19:26:23 | ... = ... | app/controllers/users_controller.rb:26:19:26:23 | "U14" |
 | app/models/user.rb:4:5:4:28 | call to update | app/models/user.rb:4:23:4:27 | "U15" |
 | app/models/user.rb:5:5:5:23 | call to update | app/models/user.rb:5:18:5:22 | "U16" |
 | app/models/user.rb:6:5:6:56 | call to update_attributes | app/models/user.rb:6:35:6:39 | "U17" |
 | app/models/user.rb:6:5:6:56 | call to update_attributes | app/models/user.rb:6:51:6:54 | true |
 | app/models/user.rb:9:5:9:40 | call to update_attribute | app/models/user.rb:9:35:9:39 | "U18" |
-| app/models/user.rb:12:5:12:13 | ... = ... | app/models/user.rb:12:17:12:21 | "U19" |
+| app/models/user.rb:12:17:12:21 | ... = ... | app/models/user.rb:12:17:12:21 | "U19" |
--- a/ruby/ql/test/library-tests/controlflow/graph/Cfg.expected
+++ b/ruby/ql/test/library-tests/controlflow/graph/Cfg.expected
@@ -3609,24 +3609,24 @@ desugar.rb:
 #    6| call to foo
 #-----|  -> __synth__0

-#    6| ... = ...
-#-----|  -> call to count=
-
 #    6| __synth__0
 #-----|  -> ...

-#    6| __synth__0
-#-----|  -> 1
-
 #    6| call to count=
 #-----|  -> __synth__0

 #    6| ...
 #-----|  -> exit m2 (normal)

+#    6| ... = ...
+#-----|  -> call to count=
+
 #    6| 1
 #-----|  -> ... = ...

+#    6| __synth__0
+#-----|  -> 1
+
 #    9| enter m3
 #-----|  -> x

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

-#   10| ... = ...
-#-----|  -> call to []=
-
 #   10| __synth__0
 #-----|  -> ...

-#   10| __synth__0
-#-----|  -> 1
-
 #   10| call to []=
 #-----|  -> __synth__0

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

+#   10| ... = ...
+#-----|  -> call to []=
+
 #   10| 1
 #-----|  -> ... = ...

+#   10| __synth__0
+#-----|  -> 1
+
 #   13| enter m4
 #-----|  -> x

--- a/ruby/ql/test/library-tests/controlflow/graph/Nodes.expected
+++ b/ruby/ql/test/library-tests/controlflow/graph/Nodes.expected
@@ -199,9 +199,9 @@ positionalArguments
 | cfg.rb:197:3:197:13 | call to bar | cfg.rb:197:7:197:7 | b |
 | cfg.rb:197:3:197:13 | call to bar | cfg.rb:197:10:197:12 | ... |
 | desugar.rb:2:5:2:6 | ... + ... | desugar.rb:2:8:2:8 | 1 |
-| desugar.rb:6:3:6:13 | call to count= | desugar.rb:6:3:6:13 | ... = ... |
-| desugar.rb:10:3:10:10 | call to []= | desugar.rb:10:3:10:10 | ... = ... |
+| desugar.rb:6:3:6:13 | call to count= | desugar.rb:6:17:6:17 | ... = ... |
 | desugar.rb:10:3:10:10 | call to []= | desugar.rb:10:9:10:9 | 0 |
+| desugar.rb:10:3:10:10 | call to []= | desugar.rb:10:14:10:14 | ... = ... |
 | desugar.rb:14:3:14:13 | call to count= | desugar.rb:14:15:14:16 | __synth__1 |
 | desugar.rb:14:15:14:16 | ... + ... | desugar.rb:14:18:14:18 | 1 |
 | desugar.rb:18:3:18:28 | call to [] | desugar.rb:18:9:18:9 | __synth__1 |
--- a/ruby/ql/test/library-tests/dataflow/type-tracker/TypeTracker.expected
+++ b/ruby/ql/test/library-tests/dataflow/type-tracker/TypeTracker.expected
@@ -50,9 +50,6 @@ track
 | 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 |
 | type_tracker.rb:14:5:14:7 | [post] var | type tracker with call steps | type_tracker.rb:7:5:9:7 | self in field |
 | type_tracker.rb:14:5:14:7 | [post] var | type tracker without call steps | type_tracker.rb:14:5:14:7 | [post] var |
-| type_tracker.rb:14:5:14:13 | ... = ... | type tracker without call steps | type_tracker.rb:14:5:14:13 | ... = ... |
-| type_tracker.rb:14:5:14:13 | [post] ... = ... | type tracker without call steps | type_tracker.rb:14:5:14:13 | [post] ... = ... |
-| type_tracker.rb:14:5:14:13 | __synth__0 | type tracker without call steps | type_tracker.rb:14:5:14:13 | __synth__0 |
 | 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= |
 | type_tracker.rb:14:17:14:23 | "hello" | type tracker with call steps | type_tracker.rb:2:16:2:18 | val |
 | 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
 | type_tracker.rb:14:17:14:23 | "hello" | type tracker without call steps | type_tracker.rb:14:17:14:23 | "hello" |
 | type_tracker.rb:14:17:14:23 | "hello" | type tracker without call steps | type_tracker.rb:15:10:15:18 | call to field |
 | 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 |
+| type_tracker.rb:14:17:14:23 | ... = ... | type tracker without call steps | type_tracker.rb:14:17:14:23 | ... = ... |
+| type_tracker.rb:14:17:14:23 | [post] ... = ... | type tracker without call steps | type_tracker.rb:14:17:14:23 | [post] ... = ... |
+| type_tracker.rb:14:17:14:23 | __synth__0 | type tracker without call steps | type_tracker.rb:14:17:14:23 | __synth__0 |
 | type_tracker.rb:15:5:15:18 | [post] self | type tracker without call steps | type_tracker.rb:15:5:15:18 | [post] self |
 | 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 |
 | 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
 | type_tracker.rb:14:5:14:7 | [post] var | type_tracker.rb:7:5:9:7 | self in field |
 | type_tracker.rb:14:5:14:7 | [post] var | type_tracker.rb:14:5:14:7 | [post] var |
 | type_tracker.rb:14:5:14:7 | [post] var | type_tracker.rb:15:10:15:12 | var |
-| type_tracker.rb:14:5:14:13 | ... = ... | type_tracker.rb:14:5:14:13 | ... = ... |
-| type_tracker.rb:14:5:14:13 | ... = ... | type_tracker.rb:14:5:14:13 | __synth__0 |
-| type_tracker.rb:14:5:14:13 | ... = ... | type_tracker.rb:14:5:14:23 | ... |
-| type_tracker.rb:14:5:14:13 | [post] ... = ... | type_tracker.rb:14:5:14:13 | [post] ... = ... |
-| type_tracker.rb:14:5:14:13 | __synth__0 | type_tracker.rb:14:5:14:13 | __synth__0 |
 | type_tracker.rb:14:5:14:13 | call to field= | type_tracker.rb:14:5:14:13 | call to field= |
 | type_tracker.rb:14:17:14:23 | "hello" | type_tracker.rb:2:16:2:18 | val |
 | type_tracker.rb:14:17:14:23 | "hello" | type_tracker.rb:2:16:2:18 | val |
 | type_tracker.rb:14:17:14:23 | "hello" | type_tracker.rb:4:9:4:20 | ... = ... |
 | type_tracker.rb:14:17:14:23 | "hello" | type_tracker.rb:4:18:4:20 | val |
-| type_tracker.rb:14:17:14:23 | "hello" | type_tracker.rb:14:5:14:13 | ... = ... |
-| type_tracker.rb:14:17:14:23 | "hello" | type_tracker.rb:14:5:14:13 | ... = ... |
 | type_tracker.rb:14:17:14:23 | "hello" | type_tracker.rb:14:5:14:13 | __synth__0 |
 | type_tracker.rb:14:17:14:23 | "hello" | type_tracker.rb:14:5:14:13 | call to field= |
 | type_tracker.rb:14:17:14:23 | "hello" | type_tracker.rb:14:5:14:23 | ... |
 | type_tracker.rb:14:17:14:23 | "hello" | type_tracker.rb:14:17:14:23 | "hello" |
+| type_tracker.rb:14:17:14:23 | "hello" | type_tracker.rb:14:17:14:23 | ... = ... |
+| type_tracker.rb:14:17:14:23 | "hello" | type_tracker.rb:14:17:14:23 | ... = ... |
 | type_tracker.rb:14:17:14:23 | "hello" | type_tracker.rb:15:10:15:18 | call to field |
+| type_tracker.rb:14:17:14:23 | ... = ... | type_tracker.rb:14:5:14:13 | __synth__0 |
+| type_tracker.rb:14:17:14:23 | ... = ... | type_tracker.rb:14:5:14:23 | ... |
+| type_tracker.rb:14:17:14:23 | ... = ... | type_tracker.rb:14:17:14:23 | ... = ... |
+| type_tracker.rb:14:17:14:23 | [post] ... = ... | type_tracker.rb:14:17:14:23 | [post] ... = ... |
+| type_tracker.rb:14:17:14:23 | __synth__0 | type_tracker.rb:14:17:14:23 | __synth__0 |
 | type_tracker.rb:15:5:15:18 | [post] self | type_tracker.rb:15:5:15:18 | [post] self |
 | type_tracker.rb:15:5:15:18 | call to puts | type_tracker.rb:12:1:16:3 | return return in m |
 | type_tracker.rb:15:5:15:18 | call to puts | type_tracker.rb:15:5:15:18 | call to puts |
--- a/ruby/ql/test/query-tests/security/cwe-295/RequestWithoutValidation.expected
+++ b/ruby/ql/test/query-tests/security/cwe-295/RequestWithoutValidation.expected
@@ -11,7 +11,7 @@
 | Httparty.rb:10:1:10:59 | call to get | This request may run with $@. | Httparty.rb:10:39:10:56 | Pair | certificate validation disabled |
 | Httparty.rb:13:1:13:70 | call to post | This request may run with $@. | Httparty.rb:13:57:13:69 | Pair | certificate validation disabled |
 | Httparty.rb:16:1:16:74 | call to post | This request may run with $@. | Httparty.rb:16:59:16:71 | Pair | certificate validation disabled |
-| NetHttp.rb:9:12:9:31 | call to request | This request may run with $@. | NetHttp.rb:7:1:7:16 | ... = ... | certificate validation disabled |
+| NetHttp.rb:9:12:9:31 | call to request | This request may run with $@. | NetHttp.rb:7:20:7:44 | ... = ... | certificate validation disabled |
 | OpenURI.rb:4:1:4:78 | call to open | This request may run with $@. | OpenURI.rb:4:36:4:77 | Pair | certificate validation disabled |
 | OpenURI.rb:7:1:7:82 | call to open | This request may run with $@. | OpenURI.rb:7:38:7:79 | Pair | certificate validation disabled |
 | OpenURI.rb:11:1:11:43 | call to open | This request may run with $@. | OpenURI.rb:10:13:10:54 | Pair | certificate validation disabled |