Skip to content

Commit 2214cae

Browse files
hmachmac
committed
Ruby: Identify named graphql params as sources
1 parent 2053ee0 commit 2214cae

File tree

3 files changed

+100
-35
lines changed

3 files changed

+100
-35
lines changed

‎ruby/ql/lib/codeql/ruby/frameworks/GraphQL.qll

Lines changed: 70 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -262,8 +262,13 @@ class GraphqlFieldDefinitionMethodCall extends GraphqlSchemaObjectClassMethodCal
262262
/**
263263
* Gets an argument call inside this field definition.
264264
*/
265-
GraphqlFieldArgumentDefinitionMethodCall getAnArgumentCall() {
266-
result.getEnclosingCallable() = this.getBlock()
265+
GraphqlFieldArgumentDefinitionMethodCall getAnArgumentCall() { result = this.getArgumentCall(_) }
266+
267+
/**
268+
* Gets the argument call for `name` inside this field definition.
269+
*/
270+
GraphqlFieldArgumentDefinitionMethodCall getArgumentCall(string name) {
271+
result.getEnclosingCallable() = this.getBlock() and result.getArgumentName() = name
267272
}
268273
}
269274

@@ -277,7 +282,7 @@ class GraphqlInputObjectArgumentDefinitionCall extends DataFlow::CallNode {
277282
.getMember("InputObject")
278283
.getADescendentModule()
279284
.getAnOwnModuleSelf()
280-
.getAMethodCall()
285+
.getAMethodCall("argument")
281286
}
282287

283288
/** Gets the name of the argument (i.e. the first argument to this `argument` method call) */
@@ -443,13 +448,16 @@ class GraphqlFieldResolutionMethod extends Method, Http::Server::RequestHandler:
443448

444449
/** Gets the method call which is the definition of the field corresponding to this resolver method. */
445450
GraphqlFieldDefinitionMethodCall getDefinition() {
446-
result
447-
.getKeywordArgument("resolver_method")
448-
.getConstantValue()
449-
.isStringlikeValue(this.getName())
450-
or
451-
not exists(result.getKeywordArgument("resolver_method").(SymbolLiteral)) and
452-
result.getFieldName() = this.getName()
451+
result.getEnclosingModule() = this.getEnclosingModule() and
452+
(
453+
result
454+
.getKeywordArgument("resolver_method")
455+
.getConstantValue()
456+
.isStringlikeValue(this.getName())
457+
or
458+
not exists(result.getKeywordArgument("resolver_method").(SymbolLiteral)) and
459+
result.getFieldName() = this.getName()
460+
)
453461
}
454462

455463
// check for a named argument the same name as a defined argument for this field
@@ -471,42 +479,78 @@ class GraphqlFieldResolutionMethod extends Method, Http::Server::RequestHandler:
471479
GraphqlSchemaObjectClass getGraphqlClass() { result = schemaObjectClass }
472480
}
473481

474-
private DataFlow::CallNode hashAccess(DataFlow::Node recv, string key) {
475-
result.asExpr() instanceof ExprNodes::ElementReferenceCfgNode and
476-
result.getArgument(0).getConstantValue().isStringlikeValue(key) and
477-
result.getReceiver() = recv
478-
}
479-
480-
private DataFlow::CallNode parameterAccess(
482+
private DataFlow::CallNode hashParameterAccess(
481483
GraphqlFieldResolutionMethod method, HashSplatParameter param, GraphqlType type
482484
) {
483-
exists(GraphqlFieldArgumentDefinitionMethodCall def, string key |
485+
exists(
486+
DataFlow::LocalSourceNode paramNode, GraphqlFieldArgumentDefinitionMethodCall def, string key
487+
|
484488
param = method.getAParameter() and
489+
paramNode.(DataFlow::ParameterNode).getParameter() = param and
485490
def = method.getDefinition().getAnArgumentCall() and
486491
(
487492
// Direct access to the params hash
488493
[def.getArgumentType(), def.getArgumentElementType()] = type and
489494
def.getArgumentName() = key and
490-
exists(DataFlow::Node paramRead |
491-
paramRead.asExpr().getExpr() = param.getVariable().getAnAccess().(VariableReadAccess) and
492-
result = hashAccess(paramRead, key)
493-
)
495+
paramNode.flowsTo(hashAccess(result, key))
494496
or
495497
// Nested access
496498
exists(GraphqlType type2 |
497-
parameterAccess(method, param, type2)
499+
hashParameterAccess(method, param, type2)
498500
.(DataFlow::LocalSourceNode)
499-
.flowsTo(result.getReceiver()) and
500-
result = hashAccess(_, key) and
501+
.flowsTo(hashAccess(result, key)) and
501502
type2.getFieldOrArgument(key) = type
502503
)
503504
)
504505
)
505506
}
506507

508+
private DataFlow::Node parameterAccess(
509+
GraphqlFieldResolutionMethod method, DataFlow::LocalSourceNode param, GraphqlType type
510+
) {
511+
param = getAGraphqlParameter(method, type) and
512+
result = param
513+
or
514+
exists(string key, GraphqlType type2 |
515+
param = parameterAccess(method, _, type2) and
516+
param.flowsTo(hashAccess(result, key)) and
517+
type2.getFieldOrArgument(key) = type
518+
)
519+
}
520+
521+
private DataFlow::ParameterNode getAGraphqlParameter(
522+
GraphqlFieldResolutionMethod method, GraphqlType type
523+
) {
524+
result.getCallable() = method and
525+
(
526+
result.getParameter() instanceof KeywordParameter and
527+
exists(GraphqlFieldArgumentDefinitionMethodCall c |
528+
c = method.getDefinition().getArgumentCall(result.getName())
529+
|
530+
type = [c.getArgumentType(), c.getArgumentElementType()]
531+
)
532+
or
533+
result.getParameter() instanceof SimpleParameter and
534+
type = method.getDefinition().getFieldType()
535+
)
536+
}
537+
538+
/**
539+
* Gets the receiver of the hash access `access` with key `key`.
540+
* For example, in `h["foo"]` the receiver is `h`, the key is "foo"
541+
* and `access` is the dataflow node for the whole expression.
542+
*/
543+
private DataFlow::Node hashAccess(DataFlow::CallNode access, string key) {
544+
access.asExpr() instanceof ExprNodes::ElementReferenceCfgNode and
545+
access.getArgument(0).getConstantValue().isStringlikeValue(key) and
546+
access.getReceiver() = result
547+
}
548+
507549
private class GraphqlParameterAccess extends RemoteFlowSource::Range {
508550
GraphqlParameterAccess() {
509-
exists(GraphqlType type | this = parameterAccess(_, _, type) and type.isScalar())
551+
exists(GraphqlType type | type.isScalar() |
552+
this = hashParameterAccess(_, _, type) or this = parameterAccess(_, _, type)
553+
)
510554
}
511555

512556
override string getSourceType() { result = "GraphQL" }

‎ruby/ql/test/library-tests/frameworks/graphql/GraphQL.expected

Lines changed: 16 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -2,21 +2,22 @@ graphqlSchemaObjectClass
22
| app/graphql/types/base_object.rb:2:3:4:5 | BaseObject |
33
| app/graphql/types/mutation_type.rb:2:3:4:5 | MutationType |
44
| app/graphql/types/post.rb:1:1:6:5 | Post |
5-
| app/graphql/types/query_type.rb:2:3:71:5 | QueryType |
5+
| app/graphql/types/query_type.rb:2:3:85:5 | QueryType |
66
graphqlSchemaObjectFieldDefinition
77
| app/graphql/types/mutation_type.rb:2:3:4:5 | MutationType | app/graphql/types/mutation_type.rb:3:5:3:44 | call to field |
88
| app/graphql/types/post.rb:1:1:6:5 | Post | app/graphql/types/post.rb:2:5:2:24 | call to field |
99
| app/graphql/types/post.rb:1:1:6:5 | Post | app/graphql/types/post.rb:3:5:3:36 | call to field |
1010
| app/graphql/types/post.rb:1:1:6:5 | Post | app/graphql/types/post.rb:4:5:4:60 | call to field |
1111
| app/graphql/types/post.rb:1:1:6:5 | Post | app/graphql/types/post.rb:5:5:5:51 | call to field |
12-
| app/graphql/types/query_type.rb:2:3:71:5 | QueryType | app/graphql/types/query_type.rb:3:5:5:40 | call to field |
13-
| app/graphql/types/query_type.rb:2:3:71:5 | QueryType | app/graphql/types/query_type.rb:7:5:9:7 | call to field |
14-
| app/graphql/types/query_type.rb:2:3:71:5 | QueryType | app/graphql/types/query_type.rb:15:5:17:7 | call to field |
15-
| app/graphql/types/query_type.rb:2:3:71:5 | QueryType | app/graphql/types/query_type.rb:24:5:26:7 | call to field |
16-
| app/graphql/types/query_type.rb:2:3:71:5 | QueryType | app/graphql/types/query_type.rb:32:5:35:7 | call to field |
17-
| app/graphql/types/query_type.rb:2:3:71:5 | QueryType | app/graphql/types/query_type.rb:46:5:49:7 | call to field |
18-
| app/graphql/types/query_type.rb:2:3:71:5 | QueryType | app/graphql/types/query_type.rb:55:5:57:7 | call to field |
19-
| app/graphql/types/query_type.rb:2:3:71:5 | QueryType | app/graphql/types/query_type.rb:65:5:67:7 | call to field |
12+
| app/graphql/types/query_type.rb:2:3:85:5 | QueryType | app/graphql/types/query_type.rb:3:5:5:40 | call to field |
13+
| app/graphql/types/query_type.rb:2:3:85:5 | QueryType | app/graphql/types/query_type.rb:7:5:9:7 | call to field |
14+
| app/graphql/types/query_type.rb:2:3:85:5 | QueryType | app/graphql/types/query_type.rb:15:5:17:7 | call to field |
15+
| app/graphql/types/query_type.rb:2:3:85:5 | QueryType | app/graphql/types/query_type.rb:24:5:26:7 | call to field |
16+
| app/graphql/types/query_type.rb:2:3:85:5 | QueryType | app/graphql/types/query_type.rb:32:5:35:7 | call to field |
17+
| app/graphql/types/query_type.rb:2:3:85:5 | QueryType | app/graphql/types/query_type.rb:46:5:49:7 | call to field |
18+
| app/graphql/types/query_type.rb:2:3:85:5 | QueryType | app/graphql/types/query_type.rb:55:5:57:7 | call to field |
19+
| app/graphql/types/query_type.rb:2:3:85:5 | QueryType | app/graphql/types/query_type.rb:65:5:67:7 | call to field |
20+
| app/graphql/types/query_type.rb:2:3:85:5 | QueryType | app/graphql/types/query_type.rb:72:5:76:7 | call to field |
2021
graphqlResolveMethod
2122
| app/graphql/mutations/dummy.rb:9:5:12:7 | resolve |
2223
| app/graphql/resolvers/dummy_resolver.rb:10:5:13:7 | resolve |
@@ -43,6 +44,7 @@ graphqlFieldDefinitionMethodCall
4344
| app/graphql/types/query_type.rb:46:5:49:7 | call to field |
4445
| app/graphql/types/query_type.rb:55:5:57:7 | call to field |
4546
| app/graphql/types/query_type.rb:65:5:67:7 | call to field |
47+
| app/graphql/types/query_type.rb:72:5:76:7 | call to field |
4648
graphqlFieldResolutionMethod
4749
| app/graphql/types/query_type.rb:10:5:13:7 | with_arg |
4850
| app/graphql/types/query_type.rb:18:5:22:7 | custom_method |
@@ -51,11 +53,13 @@ graphqlFieldResolutionMethod
5153
| app/graphql/types/query_type.rb:50:5:53:7 | with_enum |
5254
| app/graphql/types/query_type.rb:58:5:63:7 | with_nested_enum |
5355
| app/graphql/types/query_type.rb:68:5:70:7 | with_array |
56+
| app/graphql/types/query_type.rb:77:5:84:7 | with_named_params |
5457
graphqlFieldResolutionRoutedParameter
5558
| app/graphql/types/query_type.rb:10:5:13:7 | with_arg | app/graphql/types/query_type.rb:10:18:10:23 | number |
5659
| app/graphql/types/query_type.rb:18:5:22:7 | custom_method | app/graphql/types/query_type.rb:18:23:18:33 | blah_number |
5760
| app/graphql/types/query_type.rb:36:5:40:7 | with_splat_and_named_arg | app/graphql/types/query_type.rb:36:34:36:37 | arg1 |
5861
| app/graphql/types/query_type.rb:68:5:70:7 | with_array | app/graphql/types/query_type.rb:68:20:68:23 | list |
62+
| app/graphql/types/query_type.rb:77:5:84:7 | with_named_params | app/graphql/types/query_type.rb:77:27:77:30 | arg1 |
5963
graphqlFieldResolutionDefinition
6064
| app/graphql/types/query_type.rb:10:5:13:7 | with_arg | app/graphql/types/query_type.rb:7:5:9:7 | call to field |
6165
| app/graphql/types/query_type.rb:18:5:22:7 | custom_method | app/graphql/types/query_type.rb:15:5:17:7 | call to field |
@@ -64,6 +68,7 @@ graphqlFieldResolutionDefinition
6468
| 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 |
6569
| 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 |
6670
| app/graphql/types/query_type.rb:68:5:70:7 | with_array | app/graphql/types/query_type.rb:65:5:67:7 | call to field |
71+
| app/graphql/types/query_type.rb:77:5:84:7 | with_named_params | app/graphql/types/query_type.rb:72:5:76:7 | call to field |
6772
graphqlRemoteFlowSources
6873
| app/graphql/mutations/dummy.rb:5:24:5:25 | id |
6974
| app/graphql/mutations/dummy.rb:9:17:9:25 | something |
@@ -78,3 +83,5 @@ graphqlRemoteFlowSources
7883
| app/graphql/types/query_type.rb:52:22:52:32 | ...[...] |
7984
| app/graphql/types/query_type.rb:60:22:60:41 | ...[...] |
8085
| app/graphql/types/query_type.rb:68:20:68:23 | list |
86+
| app/graphql/types/query_type.rb:77:27:77:30 | arg1 |
87+
| app/graphql/types/query_type.rb:80:22:80:33 | ...[...] |

‎ruby/ql/test/library-tests/frameworks/graphql/app/graphql/types/query_type.rb

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -68,5 +68,19 @@ def with_nested_enum(**args)
6868
def with_array(list:)
6969
system("echo #{list[0]}")
7070
end
71+
72+
field :with_named_params, String do
73+
argument :arg1, String, "Arg 1"
74+
argument :arg2, Types::Post, "Arg 2"
75+
argument :arg3, Types::MediaCategory, "Arg 3"
76+
end
77+
def with_named_params(arg1:, arg2:, **args)
78+
system("echo #{arg1}")
79+
system("echo #{arg2}")
80+
system("echo #{arg2[:title]}")
81+
system("echo #{arg2[:media_category]}")
82+
system("echo #{args[:arg3]}")
83+
system("echo #{args[:not_an_arg]}")
84+
end
7185
end
7286
end

0 commit comments

Comments
 (0)