From 5706bc62051714c446148e86178a90f051bc783a Mon Sep 17 00:00:00 2001
From: Harry Maclean <hmac@github.com>
Date: Thu, 14 Sep 2023 19:02:39 +0100
Subject: [PATCH] Ruby: Model GraphQL InputObject arguments

---
 .../ql/lib/codeql/ruby/frameworks/GraphQL.qll | 82 +++++++++++++------
 .../frameworks/graphql/GraphQL.expected       | 35 ++++----
 .../graphql/app/graphql/types/direction.rb    |  6 ++
 .../graphql/app/graphql/types/post.rb         |  1 +
 .../graphql/app/graphql/types/post_order.rb   |  5 ++
 .../graphql/app/graphql/types/query_type.rb   |  1 +
 6 files changed, 89 insertions(+), 41 deletions(-)
 create mode 100644 ruby/ql/test/library-tests/frameworks/graphql/app/graphql/types/direction.rb
 create mode 100644 ruby/ql/test/library-tests/frameworks/graphql/app/graphql/types/post_order.rb

diff --git a/ruby/ql/lib/codeql/ruby/frameworks/GraphQL.qll b/ruby/ql/lib/codeql/ruby/frameworks/GraphQL.qll
index 8d16013d278f..71d40d137094 100644
--- a/ruby/ql/lib/codeql/ruby/frameworks/GraphQL.qll
+++ b/ruby/ql/lib/codeql/ruby/frameworks/GraphQL.qll
@@ -267,6 +267,29 @@ class GraphqlFieldDefinitionMethodCall extends GraphqlSchemaObjectClassMethodCal
   }
 }
 
+/**
+ * A call to `argument` in a GraphQL InputObject class.
+ */
+class GraphqlInputObjectArgumentDefinitionCall extends DataFlow::CallNode {
+  GraphqlInputObjectArgumentDefinitionCall() {
+    this =
+      graphQlSchema()
+          .getMember("InputObject")
+          .getADescendentModule()
+          .getAnOwnModuleSelf()
+          .getAMethodCall()
+  }
+
+  /** Gets the name of the argument (i.e. the first argument to this `argument` method call) */
+  string getArgumentName() { result = this.getArgument(0).getConstantValue().getStringlikeValue() }
+
+  /** Gets the type of this argument */
+  GraphqlType getArgumentType() { result = this.getArgument(1).asExpr().getExpr() }
+
+  /** Gets the class representing the receiver of this method. */
+  ClassDeclaration getReceiverClass() { result = this.asExpr().getExpr().getEnclosingModule() }
+}
+
 /**
  * A `MethodCall` that represents calling the class method `argument` inside the
  * block for a `field` definition on a GraphQL object.
@@ -313,19 +336,23 @@ private class GraphqlType extends ConstantAccess {
   Module getModule() { result.getAnImmediateReference() = this }
 
   /**
-   * Gets a field of this type, if it is an object type.
+   * Gets the type of a field/argument of this type, if it is an object type.
    */
-  GraphqlType getAField() { result = this.getField(_) }
+  GraphqlType getAFieldOrArgument() { result = this.getFieldOrArgument(_) }
 
   /**
-   * Gets the field of this type named `name`, if it exists.
+   * Gets the type of the `name` field/argument of this type, if it exists.
    */
-  GraphqlType getField(string name) {
+  GraphqlType getFieldOrArgument(string name) {
     result =
       any(GraphqlFieldDefinitionMethodCall field |
         field.getFieldName() = name and
         this.getModule().getADeclaration() = field.getReceiverClass()
-      ).getFieldType()
+      ).getFieldType() or
+    result =
+      any(GraphqlInputObjectArgumentDefinitionCall arg |
+        arg.getArgumentName() = name and this.getModule().getADeclaration() = arg.getReceiverClass()
+      ).getArgumentType()
   }
 
   /**
@@ -344,7 +371,7 @@ private class GraphqlType extends ConstantAccess {
   /**
    * Holds if this type is scalar - i.e. it is neither an object or an enum.
    */
-  predicate isScalar() { not exists(this.getAField()) and not this.isEnum() }
+  predicate isScalar() { not exists(this.getAFieldOrArgument()) and not this.isEnum() }
 }
 
 /**
@@ -440,34 +467,35 @@ private DataFlow::CallNode hashAccess(DataFlow::Node recv, string key) {
 }
 
 private DataFlow::CallNode parameterAccess(
-  GraphqlFieldResolutionMethod method, GraphqlFieldArgumentDefinitionMethodCall def,
-  HashSplatParameter param, string key, GraphqlType type
+  GraphqlFieldResolutionMethod method, HashSplatParameter param, GraphqlType type
 ) {
-  param = method.getAParameter() and
-  def = method.getDefinition().getAnArgumentCall() and
-  (
-    // Direct access to the params hash
-    def.getArgumentType() = type and
-    def.getArgumentName() = key and
-    exists(DataFlow::Node paramRead |
-      paramRead.asExpr().getExpr() = param.getVariable().getAnAccess().(VariableReadAccess) and
-      result = hashAccess(paramRead, key)
-    )
-    or
-    // Nested access
-    exists(GraphqlType type2 |
-      parameterAccess(method, _, param, _, type2)
-          .(DataFlow::LocalSourceNode)
-          .flowsTo(result.getReceiver()) and
-      result = hashAccess(_, key) and
-      type2.getField(key) = type
+  exists(GraphqlFieldArgumentDefinitionMethodCall def, string key |
+    param = method.getAParameter() and
+    def = method.getDefinition().getAnArgumentCall() and
+    (
+      // Direct access to the params hash
+      def.getArgumentType() = type and
+      def.getArgumentName() = key and
+      exists(DataFlow::Node paramRead |
+        paramRead.asExpr().getExpr() = param.getVariable().getAnAccess().(VariableReadAccess) and
+        result = hashAccess(paramRead, key)
+      )
+      or
+      // Nested access
+      exists(GraphqlType type2 |
+        parameterAccess(method, param, type2)
+            .(DataFlow::LocalSourceNode)
+            .flowsTo(result.getReceiver()) and
+        result = hashAccess(_, key) and
+        type2.getFieldOrArgument(key) = type
+      )
     )
   )
 }
 
 private class GraphqlParameterAccess extends RemoteFlowSource::Range {
   GraphqlParameterAccess() {
-    exists(GraphqlType type | this = parameterAccess(_, _, _, _, type) and type.isScalar())
+    exists(GraphqlType type | this = parameterAccess(_, _, type) and type.isScalar())
   }
 
   override string getSourceType() { result = "GraphQL" }
diff --git a/ruby/ql/test/library-tests/frameworks/graphql/GraphQL.expected b/ruby/ql/test/library-tests/frameworks/graphql/GraphQL.expected
index 25dcdc7374dd..c66be64f87e9 100644
--- a/ruby/ql/test/library-tests/frameworks/graphql/GraphQL.expected
+++ b/ruby/ql/test/library-tests/frameworks/graphql/GraphQL.expected
@@ -1,20 +1,21 @@
 graphqlSchemaObjectClass
 | app/graphql/types/base_object.rb:2:3:4:5 | BaseObject |
 | app/graphql/types/mutation_type.rb:2:3:4:5 | MutationType |
-| app/graphql/types/post.rb:1:1:5:5 | Post |
-| app/graphql/types/query_type.rb:2:3:63:5 | QueryType |
+| app/graphql/types/post.rb:1:1:6:5 | Post |
+| app/graphql/types/query_type.rb:2:3:64:5 | QueryType |
 graphqlSchemaObjectFieldDefinition
 | app/graphql/types/mutation_type.rb:2:3:4:5 | MutationType | app/graphql/types/mutation_type.rb:3:5:3:44 | call to field |
-| app/graphql/types/post.rb:1:1:5:5 | Post | app/graphql/types/post.rb:2:5:2:24 | call to field |
-| app/graphql/types/post.rb:1:1:5:5 | Post | app/graphql/types/post.rb:3:5:3:36 | call to field |
-| app/graphql/types/post.rb:1:1:5:5 | Post | app/graphql/types/post.rb:4:5:4:60 | call to field |
-| app/graphql/types/query_type.rb:2:3:63:5 | QueryType | app/graphql/types/query_type.rb:3:5:5:40 | call to field |
-| app/graphql/types/query_type.rb:2:3:63:5 | QueryType | app/graphql/types/query_type.rb:7:5:9:7 | call to field |
-| app/graphql/types/query_type.rb:2:3:63:5 | QueryType | app/graphql/types/query_type.rb:15:5:17:7 | call to field |
-| app/graphql/types/query_type.rb:2:3:63:5 | QueryType | app/graphql/types/query_type.rb:24:5:26:7 | call to field |
-| app/graphql/types/query_type.rb:2:3:63:5 | QueryType | app/graphql/types/query_type.rb:32:5:35:7 | call to field |
-| app/graphql/types/query_type.rb:2:3:63:5 | QueryType | app/graphql/types/query_type.rb:46:5:49:7 | call to field |
-| app/graphql/types/query_type.rb:2:3:63:5 | QueryType | app/graphql/types/query_type.rb:55:5:57:7 | call to field |
+| app/graphql/types/post.rb:1:1:6:5 | Post | app/graphql/types/post.rb:2:5:2:24 | call to field |
+| app/graphql/types/post.rb:1:1:6:5 | Post | app/graphql/types/post.rb:3:5:3:36 | call to field |
+| app/graphql/types/post.rb:1:1:6:5 | Post | app/graphql/types/post.rb:4:5:4:60 | call to field |
+| app/graphql/types/post.rb:1:1:6:5 | Post | app/graphql/types/post.rb:5:5:5:51 | call to field |
+| app/graphql/types/query_type.rb:2:3:64:5 | QueryType | app/graphql/types/query_type.rb:3:5:5:40 | call to field |
+| app/graphql/types/query_type.rb:2:3:64:5 | QueryType | app/graphql/types/query_type.rb:7:5:9:7 | call to field |
+| app/graphql/types/query_type.rb:2:3:64:5 | QueryType | app/graphql/types/query_type.rb:15:5:17:7 | call to field |
+| app/graphql/types/query_type.rb:2:3:64:5 | QueryType | app/graphql/types/query_type.rb:24:5:26:7 | call to field |
+| app/graphql/types/query_type.rb:2:3:64:5 | QueryType | app/graphql/types/query_type.rb:32:5:35:7 | call to field |
+| app/graphql/types/query_type.rb:2:3:64:5 | QueryType | app/graphql/types/query_type.rb:46:5:49:7 | call to field |
+| app/graphql/types/query_type.rb:2:3:64:5 | QueryType | app/graphql/types/query_type.rb:55:5:57:7 | call to field |
 graphqlResolveMethod
 | app/graphql/mutations/dummy.rb:9:5:12:7 | resolve |
 | app/graphql/resolvers/dummy_resolver.rb:10:5:13:7 | resolve |
@@ -32,6 +33,7 @@ graphqlFieldDefinitionMethodCall
 | app/graphql/types/post.rb:2:5:2:24 | call to field |
 | app/graphql/types/post.rb:3:5:3:36 | call to field |
 | app/graphql/types/post.rb:4:5:4:60 | call to field |
+| app/graphql/types/post.rb:5:5:5:51 | call to field |
 | app/graphql/types/query_type.rb:3:5:5:40 | call to field |
 | app/graphql/types/query_type.rb:7:5:9:7 | call to field |
 | app/graphql/types/query_type.rb:15:5:17:7 | call to field |
@@ -45,7 +47,7 @@ graphqlFieldResolutionMethod
 | app/graphql/types/query_type.rb:27:5:30:7 | with_splat |
 | app/graphql/types/query_type.rb:36:5:40:7 | with_splat_and_named_arg |
 | app/graphql/types/query_type.rb:50:5:53:7 | with_enum |
-| app/graphql/types/query_type.rb:58:5:62:7 | with_nested_enum |
+| app/graphql/types/query_type.rb:58:5:63:7 | with_nested_enum |
 graphqlFieldResolutionRoutedParameter
 | app/graphql/types/query_type.rb:10:5:13:7 | with_arg | app/graphql/types/query_type.rb:10:18:10:23 | number |
 | app/graphql/types/query_type.rb:18:5:22:7 | custom_method | app/graphql/types/query_type.rb:18:23:18:33 | blah_number |
@@ -56,7 +58,7 @@ graphqlFieldResolutionDefinition
 | app/graphql/types/query_type.rb:27:5:30:7 | with_splat | app/graphql/types/query_type.rb:24:5:26:7 | call to field |
 | app/graphql/types/query_type.rb:36:5:40:7 | with_splat_and_named_arg | app/graphql/types/query_type.rb:32:5:35:7 | call to field |
 | app/graphql/types/query_type.rb:50:5:53:7 | with_enum | app/graphql/types/query_type.rb:46:5:49:7 | call to field |
-| app/graphql/types/query_type.rb:58:5:62:7 | with_nested_enum | app/graphql/types/query_type.rb:55:5:57:7 | call to field |
+| app/graphql/types/query_type.rb:58:5:63:7 | with_nested_enum | app/graphql/types/query_type.rb:55:5:57:7 | call to field |
 graphqlRemoteFlowSources
 | app/graphql/mutations/dummy.rb:5:24:5:25 | id |
 | app/graphql/mutations/dummy.rb:9:17:9:25 | something |
@@ -64,4 +66,9 @@ graphqlRemoteFlowSources
 | app/graphql/resolvers/dummy_resolver.rb:10:17:10:25 | something |
 | app/graphql/types/query_type.rb:10:18:10:23 | number |
 | app/graphql/types/query_type.rb:18:23:18:33 | blah_number |
+| app/graphql/types/query_type.rb:28:22:28:37 | ...[...] |
+| app/graphql/types/query_type.rb:29:7:29:22 | ...[...] |
 | app/graphql/types/query_type.rb:36:34:36:37 | arg1 |
+| app/graphql/types/query_type.rb:38:22:38:32 | ...[...] |
+| app/graphql/types/query_type.rb:52:22:52:32 | ...[...] |
+| app/graphql/types/query_type.rb:60:22:60:41 | ...[...] |
diff --git a/ruby/ql/test/library-tests/frameworks/graphql/app/graphql/types/direction.rb b/ruby/ql/test/library-tests/frameworks/graphql/app/graphql/types/direction.rb
new file mode 100644
index 000000000000..9cc58cf1d937
--- /dev/null
+++ b/ruby/ql/test/library-tests/frameworks/graphql/app/graphql/types/direction.rb
@@ -0,0 +1,6 @@
+module Types
+    class Direction < Types::BaseEnum
+        value "asc", "Ascending order", value: "asc"
+        value "desc", "Descending order", value: "desc"
+    end
+end
\ No newline at end of file
diff --git a/ruby/ql/test/library-tests/frameworks/graphql/app/graphql/types/post.rb b/ruby/ql/test/library-tests/frameworks/graphql/app/graphql/types/post.rb
index c9d26c2148e4..316d29ff16c1 100644
--- a/ruby/ql/test/library-tests/frameworks/graphql/app/graphql/types/post.rb
+++ b/ruby/ql/test/library-tests/frameworks/graphql/app/graphql/types/post.rb
@@ -2,6 +2,7 @@ class Types::Post < GraphQL::Schema::Object
     field :title, String
     field :body, String, null: false
     field :media_category, Types::MediaCategory, null: false
+    field :direction, Types::Direction, null: false
   end
 end
   
\ No newline at end of file
diff --git a/ruby/ql/test/library-tests/frameworks/graphql/app/graphql/types/post_order.rb b/ruby/ql/test/library-tests/frameworks/graphql/app/graphql/types/post_order.rb
new file mode 100644
index 000000000000..4ec6b3edc7f0
--- /dev/null
+++ b/ruby/ql/test/library-tests/frameworks/graphql/app/graphql/types/post_order.rb
@@ -0,0 +1,5 @@
+module Types
+    class PostOrder < Types::BaseInputObject
+        argument :direction, Types::Direction, "The ordering direction", required: true
+    end
+end
\ No newline at end of file
diff --git a/ruby/ql/test/library-tests/frameworks/graphql/app/graphql/types/query_type.rb b/ruby/ql/test/library-tests/frameworks/graphql/app/graphql/types/query_type.rb
index 4d421949e0cb..e9c84e088e71 100644
--- a/ruby/ql/test/library-tests/frameworks/graphql/app/graphql/types/query_type.rb
+++ b/ruby/ql/test/library-tests/frameworks/graphql/app/graphql/types/query_type.rb
@@ -59,6 +59,7 @@ def with_nested_enum(**args)
       system("echo #{args[:inner]}")
       system("echo #{args[:inner][:title]}")
       system("echo #{args[:inner][:media_category]}")
+      system("echo #{args[:inner][:direction]}")
     end
   end
 end