Skip to content

Commit e58676c

Browse files
authored
Merge commit from fork
[2.4] Fix CVE-2025-27407
2 parents e95c424 + aa0eff7 commit e58676c

33 files changed

+350
-125
lines changed

.rubocop.yml

+5
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
@@ -41,6 +42,10 @@ Development/NoneWithoutBlockCop:
4142
- "lib/**/*"
4243
- "spec/**/*"
4344

45+
Development/NoEvalCop:
46+
Include:
47+
- "lib/**/*"
48+
4449
Development/NoFocusCop:
4550
Include:
4651
- "spec/**/*"

benchmark/run.rb

+7-2
Original file line numberDiff line numberDiff line change
@@ -112,12 +112,16 @@ def self.build_large_schema
112112
end
113113

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

123127
end
@@ -154,8 +158,9 @@ def self.profile_boot
154158
end
155159
StackProf::Report.new(result).print_text
156160

161+
retained_schema = nil
157162
report = MemoryProfiler.report do
158-
build_large_schema
163+
retained_schema = build_large_schema
159164
end
160165

161166
report.pretty_print

cop/development/no_eval_cop.rb

+18
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/analysis/analyzer.rb

+2-1
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,7 @@ def result
4242
raise GraphQL::RequiredImplementationMissingError
4343
end
4444

45+
# rubocop:disable Development/NoEvalCop This eval takes static inputs at load-time
4546
class << self
4647
private
4748

@@ -72,7 +73,7 @@ def on_leave_#{member_name}(node, parent, visitor)
7273
build_visitor_hooks :variable_definition
7374
build_visitor_hooks :variable_identifier
7475
build_visitor_hooks :abstract_node
75-
76+
# rubocop:enable Development/NoEvalCop
7677
protected
7778

7879
# @return [GraphQL::Query, GraphQL::Execution::Multiplex] Whatever this analyzer is analyzing

lib/graphql/analysis/visitor.rb

+2
Original file line numberDiff line numberDiff line change
@@ -69,6 +69,7 @@ def response_path
6969
@response_path.dup
7070
end
7171

72+
# rubocop:disable Development/NoEvalCop This eval takes static inputs at load-time
7273
# Visitor Hooks
7374
[
7475
:operation_definition, :fragment_definition,
@@ -93,6 +94,7 @@ def call_on_leave_#{node_type}(node, parent)
9394
9495
RUBY
9596
end
97+
# rubocop:enable Development/NoEvalCop
9698

9799
def on_operation_definition(node, parent)
98100
check_timeout

lib/graphql/language/nodes.rb

+3
Original file line numberDiff line numberDiff line change
@@ -141,6 +141,8 @@ def merge!(new_options)
141141
end
142142

143143
class << self
144+
# rubocop:disable Development/NoEvalCop This eval takes static inputs at load-time
145+
144146
# Add a default `#visit_method` and `#children_method_name` using the class name
145147
def inherited(child_class)
146148
super
@@ -343,6 +345,7 @@ def marshal_load(values)
343345
RUBY
344346
end
345347
end
348+
# rubocop:enable Development/NoEvalCop
346349
end
347350
end
348351

lib/graphql/language/static_visitor.rb

+37-33
Original file line numberDiff line numberDiff line change
@@ -22,39 +22,6 @@ def visit
2222
end
2323
end
2424

25-
# We don't use `alias` here because it breaks `super`
26-
def self.make_visit_methods(ast_node_class)
27-
node_method = ast_node_class.visit_method
28-
children_of_type = ast_node_class.children_of_type
29-
child_visit_method = :"#{node_method}_children"
30-
31-
class_eval(<<-RUBY, __FILE__, __LINE__ + 1)
32-
# The default implementation for visiting an AST node.
33-
# It doesn't _do_ anything, but it continues to visiting the node's children.
34-
# To customize this hook, override one of its make_visit_methods (or the base method?)
35-
# in your subclasses.
36-
#
37-
# @param node [GraphQL::Language::Nodes::AbstractNode] the node being visited
38-
# @param parent [GraphQL::Language::Nodes::AbstractNode, nil] the previously-visited node, or `nil` if this is the root node.
39-
# @return [void]
40-
def #{node_method}(node, parent)
41-
#{
42-
if method_defined?(child_visit_method)
43-
"#{child_visit_method}(node)"
44-
elsif children_of_type
45-
children_of_type.map do |child_accessor, child_class|
46-
"node.#{child_accessor}.each do |child_node|
47-
#{child_class.visit_method}(child_node, node)
48-
end"
49-
end.join("\n")
50-
else
51-
""
52-
end
53-
}
54-
end
55-
RUBY
56-
end
57-
5825
def on_document_children(document_node)
5926
document_node.children.each do |child_node|
6027
visit_method = child_node.visit_method
@@ -123,6 +90,41 @@ def on_argument_children(new_node)
12390
end
12491
end
12592

93+
# rubocop:disable Development/NoEvalCop This eval takes static inputs at load-time
94+
95+
# We don't use `alias` here because it breaks `super`
96+
def self.make_visit_methods(ast_node_class)
97+
node_method = ast_node_class.visit_method
98+
children_of_type = ast_node_class.children_of_type
99+
child_visit_method = :"#{node_method}_children"
100+
101+
class_eval(<<-RUBY, __FILE__, __LINE__ + 1)
102+
# The default implementation for visiting an AST node.
103+
# It doesn't _do_ anything, but it continues to visiting the node's children.
104+
# To customize this hook, override one of its make_visit_methods (or the base method?)
105+
# in your subclasses.
106+
#
107+
# @param node [GraphQL::Language::Nodes::AbstractNode] the node being visited
108+
# @param parent [GraphQL::Language::Nodes::AbstractNode, nil] the previously-visited node, or `nil` if this is the root node.
109+
# @return [void]
110+
def #{node_method}(node, parent)
111+
#{
112+
if method_defined?(child_visit_method)
113+
"#{child_visit_method}(node)"
114+
elsif children_of_type
115+
children_of_type.map do |child_accessor, child_class|
116+
"node.#{child_accessor}.each do |child_node|
117+
#{child_class.visit_method}(child_node, node)
118+
end"
119+
end.join("\n")
120+
else
121+
""
122+
end
123+
}
124+
end
125+
RUBY
126+
end
127+
126128
[
127129
Language::Nodes::Argument,
128130
Language::Nodes::Directive,
@@ -162,6 +164,8 @@ def on_argument_children(new_node)
162164
].each do |ast_node_class|
163165
make_visit_methods(ast_node_class)
164166
end
167+
168+
# rubocop:disable Development/NoEvalCop
165169
end
166170
end
167171
end

lib/graphql/language/visitor.rb

+59-55
Original file line numberDiff line numberDiff line change
@@ -61,61 +61,6 @@ def visit
6161
end
6262
end
6363

64-
# We don't use `alias` here because it breaks `super`
65-
def self.make_visit_methods(ast_node_class)
66-
node_method = ast_node_class.visit_method
67-
children_of_type = ast_node_class.children_of_type
68-
child_visit_method = :"#{node_method}_children"
69-
70-
class_eval(<<-RUBY, __FILE__, __LINE__ + 1)
71-
# The default implementation for visiting an AST node.
72-
# It doesn't _do_ anything, but it continues to visiting the node's children.
73-
# To customize this hook, override one of its make_visit_methods (or the base method?)
74-
# in your subclasses.
75-
#
76-
# @param node [GraphQL::Language::Nodes::AbstractNode] the node being visited
77-
# @param parent [GraphQL::Language::Nodes::AbstractNode, nil] the previously-visited node, or `nil` if this is the root node.
78-
# @return [Array, nil] If there were modifications, it returns an array of new nodes, otherwise, it returns `nil`.
79-
def #{node_method}(node, parent)
80-
if node.equal?(DELETE_NODE)
81-
# This might be passed to `super(DELETE_NODE, ...)`
82-
# by a user hook, don't want to keep visiting in that case.
83-
[node, parent]
84-
else
85-
new_node = node
86-
#{
87-
if method_defined?(child_visit_method)
88-
"new_node = #{child_visit_method}(new_node)"
89-
elsif children_of_type
90-
children_of_type.map do |child_accessor, child_class|
91-
"node.#{child_accessor}.each do |child_node|
92-
new_child_and_node = #{child_class.visit_method}_with_modifications(child_node, new_node)
93-
# Reassign `node` in case the child hook makes a modification
94-
if new_child_and_node.is_a?(Array)
95-
new_node = new_child_and_node[1]
96-
end
97-
end"
98-
end.join("\n")
99-
else
100-
""
101-
end
102-
}
103-
104-
if new_node.equal?(node)
105-
[node, parent]
106-
else
107-
[new_node, parent]
108-
end
109-
end
110-
end
111-
112-
def #{node_method}_with_modifications(node, parent)
113-
new_node_and_new_parent = #{node_method}(node, parent)
114-
apply_modifications(node, parent, new_node_and_new_parent)
115-
end
116-
RUBY
117-
end
118-
11964
def on_document_children(document_node)
12065
new_node = document_node
12166
document_node.children.each do |child_node|
@@ -216,6 +161,63 @@ def on_argument_children(new_node)
216161
new_node
217162
end
218163

164+
# rubocop:disable Development/NoEvalCop This eval takes static inputs at load-time
165+
166+
# We don't use `alias` here because it breaks `super`
167+
def self.make_visit_methods(ast_node_class)
168+
node_method = ast_node_class.visit_method
169+
children_of_type = ast_node_class.children_of_type
170+
child_visit_method = :"#{node_method}_children"
171+
172+
class_eval(<<-RUBY, __FILE__, __LINE__ + 1)
173+
# The default implementation for visiting an AST node.
174+
# It doesn't _do_ anything, but it continues to visiting the node's children.
175+
# To customize this hook, override one of its make_visit_methods (or the base method?)
176+
# in your subclasses.
177+
#
178+
# @param node [GraphQL::Language::Nodes::AbstractNode] the node being visited
179+
# @param parent [GraphQL::Language::Nodes::AbstractNode, nil] the previously-visited node, or `nil` if this is the root node.
180+
# @return [Array, nil] If there were modifications, it returns an array of new nodes, otherwise, it returns `nil`.
181+
def #{node_method}(node, parent)
182+
if node.equal?(DELETE_NODE)
183+
# This might be passed to `super(DELETE_NODE, ...)`
184+
# by a user hook, don't want to keep visiting in that case.
185+
[node, parent]
186+
else
187+
new_node = node
188+
#{
189+
if method_defined?(child_visit_method)
190+
"new_node = #{child_visit_method}(new_node)"
191+
elsif children_of_type
192+
children_of_type.map do |child_accessor, child_class|
193+
"node.#{child_accessor}.each do |child_node|
194+
new_child_and_node = #{child_class.visit_method}_with_modifications(child_node, new_node)
195+
# Reassign `node` in case the child hook makes a modification
196+
if new_child_and_node.is_a?(Array)
197+
new_node = new_child_and_node[1]
198+
end
199+
end"
200+
end.join("\n")
201+
else
202+
""
203+
end
204+
}
205+
206+
if new_node.equal?(node)
207+
[node, parent]
208+
else
209+
[new_node, parent]
210+
end
211+
end
212+
end
213+
214+
def #{node_method}_with_modifications(node, parent)
215+
new_node_and_new_parent = #{node_method}(node, parent)
216+
apply_modifications(node, parent, new_node_and_new_parent)
217+
end
218+
RUBY
219+
end
220+
219221
[
220222
Language::Nodes::Argument,
221223
Language::Nodes::Directive,
@@ -256,6 +258,8 @@ def on_argument_children(new_node)
256258
make_visit_methods(ast_node_class)
257259
end
258260

261+
# rubocop:enable Development/NoEvalCop
262+
259263
private
260264

261265
def apply_modifications(node, parent, new_node_and_new_parent)

lib/graphql/schema/argument.rb

+3-5
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, comment: 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
@comment = comment
@@ -89,11 +90,8 @@ def initialize(arg_name = nil, type_expr = nil, desc = nil, required: true, type
8990
end
9091

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

lib/graphql/schema/build_from_definition.rb

+8-7
Original file line numberDiff line numberDiff line change
@@ -467,17 +467,18 @@ def build_fields(owner, field_definitions, type_resolver, default_resolve:)
467467

468468
# Don't do this for interfaces
469469
if default_resolve
470-
owner.class_eval <<-RUBY, __FILE__, __LINE__
471-
# frozen_string_literal: true
472-
def #{resolve_method_name}(**args)
473-
field_instance = self.class.get_field("#{field_definition.name}")
474-
context.schema.definition_default_resolve.call(self.class, field_instance, object, args, context)
475-
end
476-
RUBY
470+
define_field_resolve_method(owner, resolve_method_name, field_definition.name)
477471
end
478472
end
479473
end
480474

475+
def define_field_resolve_method(owner, method_name, field_name)
476+
owner.define_method(method_name) { |**args|
477+
field_instance = self.class.get_field(field_name)
478+
context.schema.definition_default_resolve.call(self.class, field_instance, object, args, context)
479+
}
480+
end
481+
481482
def build_resolve_type(lookup_hash, directives, missing_type_handler)
482483
resolve_type_proc = nil
483484
resolve_type_proc = ->(ast_node) {

lib/graphql/schema/directive.rb

+1-1
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

0 commit comments

Comments
 (0)