Skip to content

Commit d096328

Browse files
authored
Merge commit from fork
[2.0] fix CVE 2025 27407
2 parents 68d3f49 + bfe1ab6 commit d096328

27 files changed

+307
-91
lines changed

.rubocop.yml

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
require:
22
- ./cop/development/none_without_block_cop
3+
- ./cop/development/no_eval_cop
34
- ./cop/development/no_focus_cop
45
- ./lib/graphql/rubocop/graphql/default_null_true
56
- ./lib/graphql/rubocop/graphql/default_required_true
@@ -52,6 +53,10 @@ Development/NoneWithoutBlockCop:
5253
- "lib/**/*"
5354
- "spec/**/*"
5455

56+
Development/NoEvalCop:
57+
Include:
58+
- "lib/**/*"
59+
5560
Development/NoFocusCop:
5661
Include:
5762
- "spec/**/*"

benchmark/run.rb

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -111,12 +111,16 @@ def self.build_large_schema
111111
end
112112

113113
obj_ts = 100.times.map do |n|
114+
input_obj_t = Class.new(GraphQL::Schema::InputObject) do
115+
graphql_name("Input#{n}")
116+
argument :arg, String
117+
end
114118
obj_t = Class.new(GraphQL::Schema::Object) do
115119
graphql_name("Object#{n}")
116120
implements(*int_ts)
117121
20.times do |n2|
118122
field :"field#{n2}", String do
119-
argument :arg, String
123+
argument :input, input_obj_t
120124
end
121125

122126
end
@@ -153,8 +157,9 @@ def self.profile_boot
153157
end
154158
StackProf::Report.new(result).print_text
155159

160+
retained_schema = nil
156161
report = MemoryProfiler.report do
157-
build_large_schema
162+
retained_schema = build_large_schema
158163
end
159164

160165
report.pretty_print

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
@@ -138,6 +138,8 @@ def merge!(new_options)
138138
end
139139

140140
class << self
141+
# rubocop:disable Development/NoEvalCop This eval takes static inputs at load-time
142+
141143
# Add a default `#visit_method` and `#children_method_name` using the class name
142144
def inherited(child_class)
143145
super
@@ -296,6 +298,7 @@ def self.from_a(filename, line, col, #{(scalar_method_names + @children_methods.
296298
RUBY
297299
end
298300
end
301+
# rubocop:enable Development/NoEvalCop
299302
end
300303
end
301304

lib/graphql/language/visitor.rb

Lines changed: 64 additions & 61 deletions
Original file line numberDiff line numberDiff line change
@@ -76,67 +76,6 @@ def visit
7676
end
7777
end
7878

79-
# We don't use `alias` here because it breaks `super`
80-
def self.make_visit_methods(ast_node_class)
81-
node_method = ast_node_class.visit_method
82-
children_of_type = ast_node_class.children_of_type
83-
child_visit_method = :"#{node_method}_children"
84-
85-
class_eval(<<-RUBY, __FILE__, __LINE__ + 1)
86-
# The default implementation for visiting an AST node.
87-
# It doesn't _do_ anything, but it continues to visiting the node's children.
88-
# To customize this hook, override one of its make_visit_methods (or the base method?)
89-
# in your subclasses.
90-
#
91-
# For compatibility, it calls hook procs, too.
92-
# @param node [GraphQL::Language::Nodes::AbstractNode] the node being visited
93-
# @param parent [GraphQL::Language::Nodes::AbstractNode, nil] the previously-visited node, or `nil` if this is the root node.
94-
# @return [Array, nil] If there were modifications, it returns an array of new nodes, otherwise, it returns `nil`.
95-
def #{node_method}(node, parent)
96-
if node.equal?(DELETE_NODE)
97-
# This might be passed to `super(DELETE_NODE, ...)`
98-
# by a user hook, don't want to keep visiting in that case.
99-
[node, parent]
100-
else
101-
# Run hooks if there are any
102-
new_node = node
103-
no_hooks = [email protected]?(node.class)
104-
if no_hooks || begin_visit(new_node, parent)
105-
#{
106-
if method_defined?(child_visit_method)
107-
"new_node = #{child_visit_method}(new_node)"
108-
elsif children_of_type
109-
children_of_type.map do |child_accessor, child_class|
110-
"node.#{child_accessor}.each do |child_node|
111-
new_child_and_node = #{child_class.visit_method}_with_modifications(child_node, new_node)
112-
# Reassign `node` in case the child hook makes a modification
113-
if new_child_and_node.is_a?(Array)
114-
new_node = new_child_and_node[1]
115-
end
116-
end"
117-
end.join("\n")
118-
else
119-
""
120-
end
121-
}
122-
end
123-
end_visit(new_node, parent) unless no_hooks
124-
125-
if new_node.equal?(node)
126-
[node, parent]
127-
else
128-
[new_node, parent]
129-
end
130-
end
131-
end
132-
133-
def #{node_method}_with_modifications(node, parent)
134-
new_node_and_new_parent = #{node_method}(node, parent)
135-
apply_modifications(node, parent, new_node_and_new_parent)
136-
end
137-
RUBY
138-
end
139-
14079
def on_document_children(document_node)
14180
new_node = document_node
14281
document_node.children.each do |child_node|
@@ -237,6 +176,68 @@ def on_argument_children(new_node)
237176
new_node
238177
end
239178

179+
# rubocop:disable Development/NoEvalCop This eval takes static inputs at load-time
180+
181+
def self.make_visit_methods(ast_node_class)
182+
node_method = ast_node_class.visit_method
183+
children_of_type = ast_node_class.children_of_type
184+
child_visit_method = :"#{node_method}_children"
185+
186+
class_eval(<<-RUBY, __FILE__, __LINE__ + 1)
187+
# The default implementation for visiting an AST node.
188+
# It doesn't _do_ anything, but it continues to visiting the node's children.
189+
# To customize this hook, override one of its make_visit_methods (or the base method?)
190+
# in your subclasses.
191+
#
192+
# For compatibility, it calls hook procs, too.
193+
# @param node [GraphQL::Language::Nodes::AbstractNode] the node being visited
194+
# @param parent [GraphQL::Language::Nodes::AbstractNode, nil] the previously-visited node, or `nil` if this is the root node.
195+
# @return [Array, nil] If there were modifications, it returns an array of new nodes, otherwise, it returns `nil`.
196+
def #{node_method}(node, parent)
197+
if node.equal?(DELETE_NODE)
198+
# This might be passed to `super(DELETE_NODE, ...)`
199+
# by a user hook, don't want to keep visiting in that case.
200+
[node, parent]
201+
else
202+
# Run hooks if there are any
203+
new_node = node
204+
no_hooks = [email protected]?(node.class)
205+
if no_hooks || begin_visit(new_node, parent)
206+
#{
207+
if method_defined?(child_visit_method)
208+
"new_node = #{child_visit_method}(new_node)"
209+
elsif children_of_type
210+
children_of_type.map do |child_accessor, child_class|
211+
"node.#{child_accessor}.each do |child_node|
212+
new_child_and_node = #{child_class.visit_method}_with_modifications(child_node, new_node)
213+
# Reassign `node` in case the child hook makes a modification
214+
if new_child_and_node.is_a?(Array)
215+
new_node = new_child_and_node[1]
216+
end
217+
end"
218+
end.join("\n")
219+
else
220+
""
221+
end
222+
}
223+
end
224+
end_visit(new_node, parent) unless no_hooks
225+
if new_node.equal?(node)
226+
[node, parent]
227+
else
228+
[new_node, parent]
229+
end
230+
end
231+
end
232+
def #{node_method}_with_modifications(node, parent)
233+
new_node_and_new_parent = #{node_method}(node, parent)
234+
apply_modifications(node, parent, new_node_and_new_parent)
235+
end
236+
RUBY
237+
end
238+
239+
240+
240241
[
241242
Language::Nodes::Argument,
242243
Language::Nodes::Directive,
@@ -277,6 +278,8 @@ def on_argument_children(new_node)
277278
make_visit_methods(ast_node_class)
278279
end
279280

281+
# rubocop:enable Development/NoEvalCop
282+
280283
private
281284

282285
def apply_modifications(node, parent, new_node_and_new_parent)

lib/graphql/schema/argument.rb

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,7 @@ def from_resolver?
5353
def initialize(arg_name = nil, type_expr = nil, desc = nil, required: true, type: nil, name: nil, loads: nil, description: nil, ast_node: nil, default_value: NOT_CONFIGURED, as: nil, from_resolver: false, camelize: true, prepare: nil, owner:, validates: nil, directives: nil, deprecation_reason: nil, replace_null_with_default: false, &definition_block)
5454
arg_name ||= name
5555
@name = -(camelize ? Member::BuildType.camelize(arg_name.to_s) : arg_name.to_s)
56+
NameValidator.validate!(@name)
5657
@type_expr = type_expr || type
5758
@description = desc || description
5859
@null = required != true
@@ -88,11 +89,8 @@ def initialize(arg_name = nil, type_expr = nil, desc = nil, required: true, type
8889
end
8990

9091
if definition_block
91-
if definition_block.arity == 1
92-
instance_exec(self, &definition_block)
93-
else
94-
instance_eval(&definition_block)
95-
end
92+
# `self` will still be self, it will also be the first argument to the block:
93+
instance_exec(self, &definition_block)
9694
end
9795
end
9896

lib/graphql/schema/build_from_definition.rb

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

454454
# Don't do this for interfaces
455455
if default_resolve
456-
owner.class_eval <<-RUBY, __FILE__, __LINE__
457-
# frozen_string_literal: true
458-
def #{resolve_method_name}(**args)
459-
field_instance = self.class.get_field("#{field_definition.name}")
460-
context.schema.definition_default_resolve.call(self.class, field_instance, object, args, context)
461-
end
462-
RUBY
456+
define_field_resolve_method(owner, resolve_method_name, field_definition.name)
463457
end
464458
end
465459
end
466460

461+
def define_field_resolve_method(owner, method_name, field_name)
462+
owner.define_method(method_name) { |**args|
463+
field_instance = self.class.get_field(field_name)
464+
context.schema.definition_default_resolve.call(self.class, field_instance, object, args, context)
465+
}
466+
end
467+
467468
def build_resolve_type(lookup_hash, directives, missing_type_handler)
468469
resolve_type_proc = nil
469470
resolve_type_proc = ->(ast_node) {

lib/graphql/schema/directive.rb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -99,7 +99,7 @@ def repeatable(new_value)
9999

100100
def inherited(subclass)
101101
super
102-
subclass.class_eval do
102+
subclass.class_exec do
103103
@default_graphql_name ||= nil
104104
end
105105
end

lib/graphql/schema/enum_value.rb

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

4949
if block_given?
50-
instance_eval(&block)
50+
instance_exec(self, &block)
5151
end
5252
end
5353

lib/graphql/schema/field.rb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -233,7 +233,7 @@ def initialize(type: nil, name: nil, owner: nil, null: nil, description: NOT_CON
233233

234234
@underscored_name = -Member::BuildType.underscore(name_s)
235235
@name = -(camelize ? Member::BuildType.camelize(name_s) : name_s)
236-
236+
NameValidator.validate!(@name)
237237
@description = description
238238
@type = @owner_type = @own_validators = @own_directives = @own_arguments = @arguments_statically_coercible = nil # these will be prepared later if necessary
239239

lib/graphql/schema/input_object.rb

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -131,12 +131,7 @@ def argument(*args, **kwargs, &block)
131131
end
132132
end
133133
# Add a method access
134-
method_name = argument_defn.keyword
135-
class_eval <<-RUBY, __FILE__, __LINE__
136-
def #{method_name}
137-
self[#{method_name.inspect}]
138-
end
139-
RUBY
134+
define_accessor_method(argument_defn.keyword)
140135
argument_defn
141136
end
142137

@@ -242,6 +237,13 @@ def coerce_result(value, ctx)
242237

243238
result
244239
end
240+
241+
private
242+
243+
def define_accessor_method(method_name)
244+
define_method(method_name) { self[method_name] }
245+
alias_method(method_name, method_name)
246+
end
245247
end
246248

247249
private

lib/graphql/schema/member/has_directives.rb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ class Member
66
module HasDirectives
77
def self.extended(child_cls)
88
super
9-
child_cls.module_eval { self.own_directives = nil }
9+
child_cls.module_exec { self.own_directives = nil }
1010
end
1111

1212
def inherited(child_cls)

lib/graphql/schema/member/has_fields.rb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -180,7 +180,7 @@ def self.extended(child_class)
180180

181181
def inherited(subclass)
182182
super
183-
subclass.class_eval do
183+
subclass.class_exec do
184184
@own_fields ||= nil
185185
@field_class ||= nil
186186
end

lib/graphql/schema/member/has_interfaces.rb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -119,7 +119,7 @@ def self.extended(child_class)
119119

120120
def inherited(subclass)
121121
super
122-
subclass.class_eval do
122+
subclass.class_exec do
123123
@own_interface_type_memberships ||= nil
124124
end
125125
end

lib/graphql/schema/member/type_system_helpers.rb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,7 @@ def kind
4343
private
4444

4545
def inherited(subclass)
46-
subclass.class_eval do
46+
subclass.class_exec do
4747
@to_non_null_type ||= nil
4848
@to_list_type ||= nil
4949
end

lib/graphql/tracing/appoptics_trace.rb

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,8 @@ def self.version
2828
Gem::Version.new('1.0.0')
2929
end
3030

31+
# rubocop:disable Development/NoEvalCop This eval takes static inputs at load-time
32+
3133
[
3234
'lex',
3335
'parse',
@@ -55,6 +57,8 @@ def #{trace_method}(**data)
5557
RUBY
5658
end
5759

60+
# rubocop:enable Development/NoEvalCop
61+
5862
def execute_field(query:, field:, ast_node:, arguments:, object:)
5963
return_type = field.type.unwrap
6064
trace_field = if return_type.kind.scalar? || return_type.kind.enum?

0 commit comments

Comments
 (0)