From eba086ef084507cc0fcf366e91772bddea8a5e84 Mon Sep 17 00:00:00 2001 From: Bibek Shrestha Date: Mon, 3 Apr 2017 10:40:44 -0400 Subject: [PATCH] Add more checks to Readme --- README.txt | 72 ++++++++++++++++++++++----------- test.rb | 114 ----------------------------------------------------- 2 files changed, 50 insertions(+), 136 deletions(-) delete mode 100644 test.rb diff --git a/README.txt b/README.txt index fe1696c..cab47a3 100644 --- a/README.txt +++ b/README.txt @@ -1,17 +1,51 @@ -= blah += SafeParser -home :: https://github.com/bibstha/ruby_hash_parser -code :: https://github.com/bibstha/ruby_hash_parser -bugs :: https://github.com/bibstha/ruby_hash_parser -... etc ... +home :: https://github.com/bibstha/safe_parser +code :: https://github.com/bibstha/safe_parser +bugs :: https://github.com/bibstha/safe_parser == DESCRIPTION: -Parses a hash string of the format `'{ :a => "something" }'` into an actual ruby hash object `{ a: "something" }`. -This is useful when you by mistake serialize hashes and save it in database column or a text file and you want to -convert them back to hashes without the security issues of executing `eval(hash_string)`. +Parses a ruby literal from string to its ruby value. -By default only following classes are allowed to be deserialized: +Eg: + +``` +val = SafeParser.new.safe_load('"this is a string"') +assert_equal "this is a string", val + +val = SafeParser.new.safe_load(':my_symbol') +assert_equal :my_symbol, val + +val = SafeParser.new.safe_load('123') +assert_equal 123, val + +val = SafeParser.new.safe_load('nil') +assert_nil val + +val = SafeParser.new.safe_load('true') +assert val + +val = SafeParser.new.safe_load('false') +refute val + +val = SafeParser.new.safe_load('[1, "my_str", :my_sym, 12.25, ["sub_array"], { test: "hash" }]') +assert_equal [1, "my_str", :my_sym, 12.25, ["sub_array"], { test: "hash" } ], val + +val = SafeParser.new.safe_load('{"key_1": "value", key_2: 123}') +assert_equal {"key_1": "value", key_2: 123 }, val + +# Raises exceptions when the ruby code has executable part +assert_raises(SafeParser::UnsafeError) do + val = SafeParser.new.safe_load('{ key: "string_with_exec#{2 + 2}" }') +end + +assert_raises(SafeParser::UnsafeError) do + val = SafeParser.new.safe_load('system("ls")') +end +``` + +Safe literals are any of the following: * TrueClass * FalseClass @@ -21,32 +55,26 @@ By default only following classes are allowed to be deserialized: * Array * Hash -A HashParser::BadHash exception is thrown if unserializable values are present. +Array and Hash can have any literals inside or another Array or Hash. -== FEATURES/PROBLEMS: - -* Any potential security issues? +If the ruby code contains anything besides the literals, it throws a `SafeHash::UnsafeError` Exception. == INSTALL: -* Add to Gemfile: `gem 'hash_parser'` +* Add to Gemfile: `gem 'safe_parser'` == DEVELOPERS: - require 'hash_parser' + require 'safe_parser' # This successfully parses the hash a = "{ :key_a => { :key_1a => 'value_1a', :key_2a => 'value_2a' }, :key_b => { :key_1b => 'value_1b' } }" - p HashParser.new.safe_load(a) + p SafeParser.new.safe_load(a) - # This throws a HashParser::BadHash exception + # This throws a SafeParser::BadHash exception a = "{ :key_a => system('ls') }" - p HashParser.new.safe_load(a) - -== TODO: - -* Allow objects of certain types to be deserialized + p SafeParser.new.safe_load(a) == LICENSE: diff --git a/test.rb b/test.rb deleted file mode 100644 index c1215e6..0000000 --- a/test.rb +++ /dev/null @@ -1,114 +0,0 @@ -require "minitest/autorun" -require "minitest/spec" -require "minitest/pride" -require "pry-byebug" -require "ruby_parser" - -# Whiltelist based parser -class HashParser - ALLOWED_CLASSES = [ :true, :false, :nil, :lit, :str, :array, :hash ].freeze - - BadHash = Class.new(StandardError) - - def safe_load(string) - raise BadHash, "#{ string } is a bad hash" unless safe?(string) - hash = {} - Thread.new do - $SAFE = 1 - hash = eval(string) - end.join - hash - end - - private - - def safe?(string) - # 1. is a hash - # 2. everything belongs to ALLOWED_CLASSES only - - expression = RubyParser.new.parse(string) - return false unless expression.head == :hash # root has to be a hash - - expression.deep_each.all? do |child| - ALLOWED_CLASSES.include?(child.head) - end - end -end - -describe HashParser do - before do - @parser = HashParser.new - end - - describe "#safe_load" do - it "fails if it is not a hash" do - strs = ["[]", "Label.delete_all", "puts 'hello'", '"#{puts 123}"', "system('rm -rf /')"] - strs.each do |bad_str| - assert_raises HashParser::BadHash, "#{ bad_str } should not be safe" do - @parser.safe_load(bad_str) - end - end - end - - it "fails if there are more than one expression, it fails" do - strs = ["{ :a => 1 }; 'hello'", "{ :a => 1 }\n{ :b => 2 }"] - strs.each do |bad_str| - assert_raises HashParser::BadHash, "#{ bad_str } should not be safe" do - @parser.safe_load(bad_str) - end - end - end - - it "Does not define or redefine any methods" do - str = '{ :a => refine(BadHash) { def safe?; "HAHAHA"; end } }' - assert_raises HashParser::BadHash, "#{ str } should not be safe" do - @parser.safe_load(str) - end - - str = '{ :a => def HashParser.safe?; "HAHAHA"; end }' - assert_raises HashParser::BadHash, "#{ str } should not be safe" do - @parser.safe_load(str) - end - end - - it "fails if it has assignment" do - strs = ['{ :a => (HashParser::TEST_CONSTANT = 1) }', - '{ :a => (hello_world = 1) }'] - strs.each do |str| - assert_raises HashParser::BadHash, "#{ str } should not be safe" do - @parser.safe_load(str) - end - end - end - - it "fails if a hash has a method call" do - strs = ["{ :a => 2 * 2 }", - "{ :a => system('rm -rf /') }", - "{ :a => Label.delete_all }", - '{ :a => "#{500}" }', - '{ :a => "#{ Label.delete_all }" }', - '{ :a => refine(BadHash) { def safe?; "HAHAHA"; end } }', - ] - strs.each do |bad_str| - assert_raises HashParser::BadHash, "#{ bad_str } should not be safe" do - @parser.safe_load(bad_str) - end - end - end - - it "passes for hashes" do - strs = ["{}", '{ "a" => "A" }', '{ :a => 123 }', '{ :a => true, :b => false, :c => true, "d" => nil }'] - parsed = [{}, { "a" => "A" }, { a: 123 }, { a: true, b: false, c: true, "d" => nil }] - strs.each.with_index do |good_str, i| - assert_equal parsed[i], @parser.safe_load(good_str), "#{ good_str } should be safe" - end - end - - it "passes for hashes with sub hashes" do - str = '{ :a => [1, 2, { "x" => "y" }] }' - parsed = { a: [1, 2, { "x" => "y" }] } - assert_equal parsed, @parser.safe_load(str) - end - end -end -