Skip to content

Commit

Permalink
security check before eval'ing block participant
Browse files Browse the repository at this point in the history
security check before eval'ing block participant block code
  • Loading branch information
jmettraux committed Jan 30, 2011
1 parent 1024662 commit 71f6fd3
Show file tree
Hide file tree
Showing 8 changed files with 85 additions and 30 deletions.
6 changes: 3 additions & 3 deletions lib/ruote/part/block_participant.rb
Expand Up @@ -82,14 +82,14 @@ def consume (workitem)

block = @opts['block']

@context.treechecker.block_check(block)
# raises in case of 'security' violation

#block = eval(block, @context.send(:binding))
# doesn't work with ruby 1.9.2-p136

block = eval(block, @context.instance_eval { binding })
# works OK with ruby 1.8.7-249 and 1.9.2-p136

# TODO : security !!

r = if block.arity == 1

block.call(workitem)
Expand Down
2 changes: 1 addition & 1 deletion lib/ruote/reader.rb
Expand Up @@ -167,7 +167,7 @@ def self.remote? (definition)
#
def ruby_eval (s)

@context.treechecker.check(s)
@context.treechecker.definition_check(s)
eval(s)

rescue Exception => e
Expand Down
2 changes: 1 addition & 1 deletion lib/ruote/svc/dollar_sub.rb
Expand Up @@ -191,7 +191,7 @@ def ruby_eval (ruby_code)
"< (http://ruote.rubyforge.org/dollar.html)"
) if @fexp.context['ruby_eval_allowed'] != true

@fexp.context.treechecker.check(ruby_code)
@fexp.context.treechecker.dollar_check(ruby_code)

RubyContext.new(self).instance_eval(ruby_code)
end
Expand Down
64 changes: 50 additions & 14 deletions lib/ruote/svc/treechecker.rb
Expand Up @@ -39,7 +39,7 @@ def initialize (context)

(context['use_ruby_treechecker'] == false) and return

@checker = Rufus::TreeChecker.new do
checker = Rufus::TreeChecker.new do

exclude_fvccall :abort, :exit, :exit!
exclude_fvccall :system, :fork, :syscall, :trap, :require, :load
Expand All @@ -48,13 +48,14 @@ def initialize (context)
#exclude_call_to :class
exclude_fvcall :private, :public, :protected

#exclude_def # no method definition
#exclude_raise # no raise or throw

exclude_def # no method definition
exclude_eval # no eval, module_eval or instance_eval
exclude_backquotes # no `rm -fR the/kitchen/sink`
exclude_alias # no alias or aliast_method
exclude_global_vars # $vars are off limits
exclude_module_tinkering # no module opening
exclude_raise # no raise or throw

exclude_rebinding Kernel # no 'k = Kernel'

Expand All @@ -69,24 +70,59 @@ def initialize (context)
exclude_call_to :instance_variable_get, :instance_variable_set
end

@cchecker = @checker.clone # and not dup
@cchecker.add_rules do
at_root do
exclude_head [ :block ] # preventing 'a < b; do_sthing_evil()'
exclude_head [ :lasgn ] # preventing 'a = 3'
end
# the checker used when reading process definitions

@def_checker = checker.clone # and not dup
@def_checker.add_rules do
exclude_raise # no raise or throw
end
@def_checker.freeze

# the checker used when dealing with BlockParticipant code

@blo_checker = checker.clone # and not dup
@blo_checker.freeze

## the checker used when dealing with conditionals
#
#@con_checker = checker.clone # and not dup
#@con_checker.add_rules do
# exclude_raise # no raise or throw
# at_root do
# exclude_head [ :block ] # preventing 'a < b; do_sthing_evil()'
# exclude_head [ :lasgn ] # preventing 'a = 3'
# end
#end
#@con_checker.freeze
#
# lib/ruote/exp/condition.rb doesn't use this treechecker
# kept (commented out) for 'documentation'

# the checker used when dealing with code in $(ruby:xxx}

@dol_checker = checker.clone # and not dup
@dol_checker.add_rules do
exclude_raise # no raise or throw
end
@dol_checker.freeze

@checker.freeze
@cchecker.freeze
freeze
#
# preventing further modifications
end

def check (ruby_code)
def definition_check (ruby_code)

@def_checker.check(ruby_code) if @def_checker
end

def block_check (ruby_code)

@blo_checker.check(ruby_code) if @blo_checker
end

def dollar_check (ruby_code)

@checker.check(ruby_code) if @checker
@dol_checker.check(ruby_code) if @dol_checker
end
end
end
Expand Down
Expand Up @@ -84,5 +84,28 @@ def test_non_jsonfiable_result

assert_match match, @tracer.to_s
end

def test_raise_security_error_before_evaluating_rogue_code

fn = "test/bad.#{Time.now.to_f}.txt"

@engine.participant_list = [
#[ 'alpha', [ 'Ruote::BlockParticipant', { 'block' => 'exit(3)' } ] ]
[ 'alpha', [ 'Ruote::BlockParticipant', { 'block' => "proc { File.open(\"#{fn}\", \"wb\") { |f| f.puts(\"bad\") } }" } ] ]
]

#noisy

wfid = @engine.launch(Ruote.define { alpha })

@engine.wait_for(wfid)

assert_equal false, File.exist?(fn), 'security check not enforced'

assert_equal 1, @engine.errors(wfid).size
assert_match /SecurityError/, @engine.errors(wfid).first.message

FileUtils.rm(fn) rescue nil
end
end

2 changes: 2 additions & 0 deletions test/functional/ft_2_errors.rb
Expand Up @@ -181,6 +181,8 @@ def test_error_in_do_no_thread_participant
raise "something went wrong" if stash[:count] == 1
end

#noisy

wfid = @engine.launch(pdef)

wait_for(wfid)
Expand Down
4 changes: 4 additions & 0 deletions test/unit/ut_0_ruby_reader.rb
Expand Up @@ -155,6 +155,10 @@ def test_treechecker
assert_raise ArgumentError do
Ruote::Reader.read %{ Ruote.define { at_exit { } } }
end

assert_raise ArgumentError do
Ruote::Reader.read %{ Ruote.define { def nada; end } }
end
end

def test_attribute_text_regexp
Expand Down
12 changes: 1 addition & 11 deletions test/unit/ut_6_condition.rb
Expand Up @@ -13,17 +13,7 @@

class ConditionTest < Test::Unit::TestCase

class Conditional

def treechecker
return @tc if @tc
@tc = Ruote::TreeChecker.new
@tc.context = {}
@tc
end
end

class FakeExpression < Conditional
class FakeExpression

def initialize (h)
@h = h
Expand Down

0 comments on commit 71f6fd3

Please sign in to comment.