Skip to content

Commit 28233b1

Browse files
authored
Merge commit from fork
[1.12] fix CVE 2025 27407
2 parents 95392ba + 6624766 commit 28233b1

9 files changed

Lines changed: 193 additions & 20 deletions

File tree

cop/development/no_eval_cop.rb

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
# frozen_string_literal: true
2+
require 'rubocop'
3+
4+
module Cop
5+
module Development
6+
class NoEvalCop < RuboCop::Cop::Base
7+
MSG_TEMPLATE = "Don't use `%{eval_method_name}` which accepts strings and may result evaluating unexpected code. Use `%{exec_method_name}` instead, and pass a block."
8+
9+
def on_send(node)
10+
case node.method_name
11+
when :module_eval, :class_eval, :instance_eval
12+
message = MSG_TEMPLATE % { eval_method_name: node.method_name, exec_method_name: node.method_name.to_s.sub("eval", "exec").to_sym }
13+
add_offense node, message: message
14+
end
15+
end
16+
end
17+
end
18+
end

lib/graphql/language/nodes.rb

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -133,6 +133,8 @@ def merge!(new_options)
133133
end
134134

135135
class << self
136+
# rubocop:disable Development/NoEvalCop This eval takes static inputs at load-time
137+
136138
# Add a default `#visit_method` and `#children_method_name` using the class name
137139
def inherited(child_class)
138140
super
@@ -276,6 +278,7 @@ def initialize_node #{arguments.join(", ")}
276278
RUBY
277279
end
278280
end
281+
# rubocop:enable Development/NoEvalCop
279282
end
280283
end
281284

lib/graphql/schema/argument.rb

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,7 @@ def from_resolver?
5555
def initialize(arg_name = nil, type_expr = nil, desc = nil, required:, type: nil, name: nil, loads: nil, description: nil, ast_node: nil, default_value: NO_DEFAULT, as: nil, from_resolver: false, camelize: true, prepare: nil, method_access: true, owner:, validates: nil, directives: nil, deprecation_reason: nil, &definition_block)
5656
arg_name ||= name
5757
@name = -(camelize ? Member::BuildType.camelize(arg_name.to_s) : arg_name.to_s)
58+
NameValidator.validate!(@name)
5859
@type_expr = type_expr || type
5960
@description = desc || description
6061
@null = !required
@@ -78,11 +79,8 @@ def initialize(arg_name = nil, type_expr = nil, desc = nil, required:, type: nil
7879
self.validates(validates)
7980

8081
if definition_block
81-
if definition_block.arity == 1
82-
instance_exec(self, &definition_block)
83-
else
84-
instance_eval(&definition_block)
85-
end
82+
# `self` will still be self, it will also be the first argument to the block:
83+
instance_exec(self, &definition_block)
8684
end
8785
end
8886

lib/graphql/schema/build_from_definition.rb

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -426,17 +426,18 @@ def build_fields(owner, field_definitions, type_resolver, default_resolve:)
426426

427427
# Don't do this for interfaces
428428
if default_resolve
429-
owner.class_eval <<-RUBY, __FILE__, __LINE__
430-
# frozen_string_literal: true
431-
def #{resolve_method_name}(**args)
432-
field_instance = self.class.get_field("#{field_definition.name}")
433-
context.schema.definition_default_resolve.call(self.class, field_instance, object, args, context)
434-
end
435-
RUBY
429+
define_field_resolve_method(owner, resolve_method_name, field_definition.name)
436430
end
437431
end
438432
end
439433

434+
def define_field_resolve_method(owner, method_name, field_name)
435+
owner.define_method(method_name) { |**args|
436+
field_instance = self.class.get_field(field_name)
437+
context.schema.definition_default_resolve.call(self.class, field_instance, object, args, context)
438+
}
439+
end
440+
440441
def build_resolve_type(lookup_hash, directives, missing_type_handler)
441442
resolve_type_proc = nil
442443
resolve_type_proc = ->(ast_node) {

lib/graphql/schema/enum_value.rb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,7 @@ def initialize(graphql_name, desc = nil, owner:, ast_node: nil, directives: nil,
5555
end
5656

5757
if block_given?
58-
instance_eval(&block)
58+
instance_exec(self, &block)
5959
end
6060
end
6161

lib/graphql/schema/field.rb

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -231,6 +231,7 @@ def initialize(type: nil, name: nil, owner: nil, null: nil, field: nil, function
231231
name_s = -name.to_s
232232
@underscored_name = -Member::BuildType.underscore(name_s)
233233
@name = -(camelize ? Member::BuildType.camelize(name_s) : name_s)
234+
NameValidator.validate!(@name)
234235
@description = description
235236
if field.is_a?(GraphQL::Schema::Field)
236237
raise ArgumentError, "Instead of passing a field as `field:`, use `add_field(field)` to add an already-defined field."

lib/graphql/schema/input_object.rb

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -123,12 +123,8 @@ class << self
123123
def argument(*args, **kwargs, &block)
124124
argument_defn = super(*args, **kwargs, &block)
125125
# Add a method access
126-
method_name = argument_defn.keyword
127-
class_eval <<-RUBY, __FILE__, __LINE__
128-
def #{method_name}
129-
self[#{method_name.inspect}]
130-
end
131-
RUBY
126+
define_accessor_method(argument_defn.keyword)
127+
argument_defn
132128
end
133129

134130
def to_graphql
@@ -236,6 +232,13 @@ def coerce_result(value, ctx)
236232

237233
result
238234
end
235+
236+
private
237+
238+
def define_accessor_method(method_name)
239+
define_method(method_name) { self[method_name] }
240+
alias_method(method_name, method_name)
241+
end
239242
end
240243

241244
private

lib/graphql/version.rb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
11
# frozen_string_literal: true
22
module GraphQL
3-
VERSION = "1.12.24"
3+
VERSION = "1.12.25"
44
end

spec/graphql/schema/loader_spec.rb

Lines changed: 149 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -389,4 +389,153 @@ def assert_deep_equal(expected_type, actual_type)
389389
assert_equal arg.default_value, { 'id' => nil, 'int' => nil, 'float' => nil, 'enum' => nil, 'sub' => nil, 'bool' => nil, 'bigint' => nil }
390390
end
391391
end
392+
393+
it "validates field argument names" do
394+
json = {
395+
"data" => {
396+
"__schema" => {
397+
"queryType" => {
398+
"name" => "Query"
399+
},
400+
"mutationType" => nil,
401+
"subscriptionType" => nil,
402+
"types" => [
403+
{
404+
"kind" => "OBJECT",
405+
"name" => "Query",
406+
"description" => nil,
407+
"fields" => [
408+
{
409+
"name" => "int",
410+
"description" => nil,
411+
"type" => {
412+
"kind" => "SCALAR",
413+
"name" => "Int",
414+
"ofType" => nil,
415+
},
416+
"args" => [
417+
{
418+
"name" => "something-wrong",
419+
"description" => nil,
420+
"type" => {
421+
"kind" => "SCALAR",
422+
"name" => "Int",
423+
"ofType" => nil
424+
},
425+
"defaultValue" => nil
426+
}
427+
],
428+
}
429+
]
430+
}
431+
]
432+
}
433+
}
434+
}
435+
err = assert_raises GraphQL::InvalidNameError do
436+
GraphQL::Schema.from_introspection(json)
437+
end
438+
439+
assert_includes err.message, "something-wrong"
440+
end
441+
442+
it "validates field names" do
443+
json = {
444+
"data" => {
445+
"__schema" => {
446+
"queryType" => {
447+
"name" => "Query"
448+
},
449+
"mutationType" => nil,
450+
"subscriptionType" => nil,
451+
"types" => [
452+
{
453+
"kind" => "OBJECT",
454+
"name" => "Query",
455+
"description" => nil,
456+
"fields" => [
457+
{
458+
"name" => "bad.int",
459+
"description" => nil,
460+
"type" => {
461+
"kind" => "SCALAR",
462+
"name" => "Int",
463+
"ofType" => nil,
464+
},
465+
"args" => [],
466+
}
467+
]
468+
}
469+
]
470+
}
471+
}
472+
}
473+
err = assert_raises GraphQL::InvalidNameError do
474+
GraphQL::Schema.from_introspection(json)
475+
end
476+
477+
assert_includes err.message, "bad.int"
478+
end
479+
480+
it "validates input object argument names" do
481+
json = {
482+
"data" => {
483+
"__schema" => {
484+
"queryType" => {
485+
"name" => "Query"
486+
},
487+
"mutationType" => nil,
488+
"subscriptionType" => nil,
489+
"types" => [
490+
{
491+
"kind" => "OBJECT",
492+
"name" => "Query",
493+
"description" => nil,
494+
"fields" => [
495+
{
496+
"name" => "int",
497+
"description" => nil,
498+
"type" => {
499+
"kind" => "SCALAR",
500+
"name" => "Int",
501+
"ofType" => nil,
502+
},
503+
"args" => [
504+
{
505+
"name" => "inputObject",
506+
"description" => nil,
507+
"type" => {
508+
"kind" => "INPUT_OBJECT",
509+
"name" => "SomeInputObject",
510+
"ofType" => nil
511+
},
512+
"defaultValue" => nil
513+
}
514+
],
515+
}
516+
]
517+
},
518+
{
519+
"kind" => "INPUT_OBJECT",
520+
"name" => "SomeInputObject",
521+
"description" => nil,
522+
"inputFields" => [
523+
{
524+
"name"=>"bad, input",
525+
"type"=> { "kind" => "SCALAR", "name" => "String"},
526+
"defaultValue"=> nil,
527+
"description" => nil,
528+
},
529+
]
530+
}
531+
]
532+
}
533+
}
534+
}
535+
err = assert_raises GraphQL::InvalidNameError do
536+
GraphQL::Schema.from_introspection(json)
537+
end
538+
539+
assert_includes err.message, "bad, input"
540+
end
392541
end

0 commit comments

Comments
 (0)