Skip to content

Commit d7859cb

Browse files
committed
Fix parsing of defined? blocks for complex expressions
YARD evaluates these blocks which can lead to arbitrary execution of code. Thanks to Nelson Elhage <[email protected]> for reporting this issue. Fixes #1177
1 parent 44ca65d commit d7859cb

File tree

4 files changed

+41
-7
lines changed

4 files changed

+41
-7
lines changed

CHANGELOG.md

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,13 @@
1+
# master
2+
3+
# [0.9.15] - July 17th, 2018
4+
5+
[0.9.15]: https://github.com/lsegal/yard/compare/v0.9.14...v0.9.15
6+
7+
- Fixed security issue in parsing of Ruby code that could allow for arbitrary
8+
execution. Credit to Nelson Elhage <[email protected]> for discovering this
9+
issue.
10+
111
# [0.9.14] - June 2nd, 2018
212

313
[0.9.14]: https://github.com/lsegal/yard/compare/v0.9.13...v0.9.14

lib/yard/handlers/ruby/class_condition_handler.rb

Lines changed: 11 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -52,12 +52,17 @@ def parse_condition
5252
# defined? keyword used, let's see if we can look up the name
5353
# in the registry, then we'll try using Ruby's powers. eval() is not
5454
# *too* dangerous here since code is not actually executed.
55-
name = statement.condition[0].source
56-
obj = YARD::Registry.resolve(namespace, name, true)
57-
begin
58-
condition = true if obj || Object.instance_eval("defined? #{name}")
59-
rescue SyntaxError, NameError
60-
condition = false
55+
arg = statement.condition.first
56+
57+
if arg.type == :var_ref
58+
name = arg.source
59+
obj = YARD::Registry.resolve(namespace, name, true)
60+
61+
begin
62+
condition = true if obj || (name && Object.instance_eval("defined? #{name}"))
63+
rescue SyntaxError, NameError
64+
condition = false
65+
end
6166
end
6267
when :var_ref
6368
var = statement.condition[0]

lib/yard/handlers/ruby/legacy/class_condition_handler.rb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,7 @@ def parse_condition
4141
case statement.tokens[1..-1].to_s.strip
4242
when /^(\d+)$/
4343
condition = $1 != "0"
44-
when /^defined\?\s*\(?(.+?)\)?$/
44+
when /^defined\?\s*\(?\s*([A-Za-z0-9:_]+?)\s*\)?$/
4545
# defined? keyword used, let's see if we can look up the name
4646
# in the registry, then we'll try using Ruby's powers. eval() is not
4747
# *too* dangerous here since code is not actually executed.

spec/handlers/class_condition_handler_spec.rb

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -65,4 +65,23 @@ def no_undoc_error(code)
6565
eof
6666
no_undoc_error "if caller.none? { |l| l =~ %r{lib/rails/generators\\.rb:(\\d+):in `lookup!'$} }; end"
6767
end
68+
69+
it "only parses identifiers or namespaces from defined? expressions" do
70+
module A
71+
@@value = nil
72+
def self.b(value) @@value = value end
73+
def self.value; @@value end
74+
end
75+
76+
YARD.parse_string <<-eof
77+
if defined? A.b(42).to_i
78+
class Foo; end
79+
else
80+
class Bar; end
81+
end
82+
eof
83+
expect(A.value).to be_nil
84+
expect(YARD::Registry.at('Foo')).not_to be_nil
85+
expect(YARD::Registry.at('Bar')).not_to be_nil
86+
end
6887
end

0 commit comments

Comments
 (0)