From 6f7e07d9988fd71e75728285a9364a14871cebf9 Mon Sep 17 00:00:00 2001 From: Bibek Shrestha Date: Thu, 30 Mar 2017 10:14:50 -0400 Subject: [PATCH] Initial commit --- .gitignore | 1 + .ruby-version | 1 + Gemfile | 4 ++ Gemfile.lock | 29 ++++++++++ History.txt | 5 ++ Manifest.txt | 6 +++ README.txt | 74 +++++++++++++++++++++++++ Rakefile | 13 +++++ lib/hash_parser.rb | 30 +++++++++++ test.rb | 114 +++++++++++++++++++++++++++++++++++++++ test/test_hash_parser.rb | 83 ++++++++++++++++++++++++++++ 11 files changed, 360 insertions(+) create mode 100644 .gitignore create mode 100644 .ruby-version create mode 100644 Gemfile create mode 100644 Gemfile.lock create mode 100644 History.txt create mode 100644 Manifest.txt create mode 100644 README.txt create mode 100644 Rakefile create mode 100644 lib/hash_parser.rb create mode 100644 test.rb create mode 100644 test/test_hash_parser.rb diff --git a/.gitignore b/.gitignore new file mode 100644 index 0000000..5fff1d9 --- /dev/null +++ b/.gitignore @@ -0,0 +1 @@ +pkg diff --git a/.ruby-version b/.ruby-version new file mode 100644 index 0000000..bed811b --- /dev/null +++ b/.ruby-version @@ -0,0 +1 @@ +ruby-2.3.3 diff --git a/Gemfile b/Gemfile new file mode 100644 index 0000000..261b208 --- /dev/null +++ b/Gemfile @@ -0,0 +1,4 @@ +source "https://rubygems.org" +gem 'ruby_parser' +gem 'pry-byebug' +gem 'minitest' diff --git a/Gemfile.lock b/Gemfile.lock new file mode 100644 index 0000000..5f5e386 --- /dev/null +++ b/Gemfile.lock @@ -0,0 +1,29 @@ +GEM + remote: https://rubygems.org/ + specs: + byebug (9.0.6) + coderay (1.1.1) + method_source (0.8.2) + minitest (5.10.1) + pry (0.10.4) + coderay (~> 1.1.0) + method_source (~> 0.8.1) + slop (~> 3.4) + pry-byebug (3.4.2) + byebug (~> 9.0) + pry (~> 0.10) + ruby_parser (3.8.4) + sexp_processor (~> 4.1) + sexp_processor (4.8.0) + slop (3.6.0) + +PLATFORMS + ruby + +DEPENDENCIES + minitest + pry-byebug + ruby_parser + +BUNDLED WITH + 1.14.6 diff --git a/History.txt b/History.txt new file mode 100644 index 0000000..cbdd055 --- /dev/null +++ b/History.txt @@ -0,0 +1,5 @@ +=== 0.0.1 / 2017-03-29 + +* Initial implementation + +The whitelist based implementation only loads the classes Psych::safe_load loads. diff --git a/Manifest.txt b/Manifest.txt new file mode 100644 index 0000000..3d8c417 --- /dev/null +++ b/Manifest.txt @@ -0,0 +1,6 @@ +History.txt +Manifest.txt +README.txt +Rakefile +lib/hash_parser.rb +test/test_hash_parser.rb diff --git a/README.txt b/README.txt new file mode 100644 index 0000000..fe1696c --- /dev/null +++ b/README.txt @@ -0,0 +1,74 @@ += blah + +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 ... + +== 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)`. + +By default only following classes are allowed to be deserialized: + +* TrueClass +* FalseClass +* NilClass +* Numeric +* String +* Array +* Hash + +A HashParser::BadHash exception is thrown if unserializable values are present. + +== FEATURES/PROBLEMS: + +* Any potential security issues? + +== INSTALL: + +* Add to Gemfile: `gem 'hash_parser'` + +== DEVELOPERS: + + require 'hash_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) + + # This throws a HashParser::BadHash exception + a = "{ :key_a => system('ls') }" + p HashParser.new.safe_load(a) + +== TODO: + +* Allow objects of certain types to be deserialized + +== LICENSE: + +(The MIT License) + +Copyright (c) 2017 FIX + +Permission is hereby granted, free of charge, to any person obtaining +a copy of this software and associated documentation files (the +'Software'), to deal in the Software without restriction, including +without limitation the rights to use, copy, modify, merge, publish, +distribute, sublicense, and/or sell copies of the Software, and to +permit persons to whom the Software is furnished to do so, subject to +the following conditions: + +The above copyright notice and this permission notice shall be +included in all copies or substantial portions of the Software. + +THE SOFTWARE IS PROVIDED 'AS IS', WITHOUT WARRANTY OF ANY KIND, +EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF +MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. +IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY +CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, +TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE +SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. diff --git a/Rakefile b/Rakefile new file mode 100644 index 0000000..881f601 --- /dev/null +++ b/Rakefile @@ -0,0 +1,13 @@ +# -*- ruby -*- + +require 'hoe' + +Hoe.spec 'hash_parser' do + developer 'Bibek Shrestha', 'bibekshrestha@gmail.com' + + license "MIT" + + dependency "ruby_parser", "~> 3.8.4" +end + +# vim: syntax=ruby diff --git a/lib/hash_parser.rb b/lib/hash_parser.rb new file mode 100644 index 0000000..065199b --- /dev/null +++ b/lib/hash_parser.rb @@ -0,0 +1,30 @@ +require "ruby_parser" + +# Whiltelist based hash string parser +class HashParser + VERSION = "0.0.1" + + # a literal is strings, regex, numeric + # https://github.com/seattlerb/ruby_parser/blob/master/lib/ruby19_parser.y#L890 + 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) + eval(string) + end + + private + + def safe?(string) + expression = RubyParser.new.parse(string) + return false unless expression.head == :hash # root has to be a hash + + # can be optimized to do an ACTUAL_CLASSES - ALLOWED_CLASSES == [] + expression.deep_each.all? do |child| + ALLOWED_CLASSES.include?(child.head) + end + end +end + diff --git a/test.rb b/test.rb new file mode 100644 index 0000000..c1215e6 --- /dev/null +++ b/test.rb @@ -0,0 +1,114 @@ +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 + diff --git a/test/test_hash_parser.rb b/test/test_hash_parser.rb new file mode 100644 index 0000000..b9d3064 --- /dev/null +++ b/test/test_hash_parser.rb @@ -0,0 +1,83 @@ +require "minitest/autorun" +require "minitest/spec" +require "minitest/pride" +require "hash_parser" + +describe HashParser do + before do + @parser = HashParser.new + end + + describe "#safe_load" do + it "fails if it is not a hash" do + strs = ["[]", "Book.delete_all", "puts 'hello'", '"#{puts 123}"', "system('ls /')"] + 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 => SOME_CONST }", + "{ :a => system('ls /') }", + "{ :a => Book.delete_all }", + '{ :a => "#{500}" }', + '{ :a => "#{ Book.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 +