Merge branch '1915-css-tokenizer-load-file-vulnerability_v1.10.x' int…

…o v1.10.x
sparklemotion · Aug 11, 2019 · 5d30128 · 5d30128
2 parents 45ee92b + c86b5fc
commit 5d30128
Show file tree

Hide file tree

Showing 6 changed files with 240 additions and 202 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -1,5 +1,18 @@
 # Nokogiri Changelog
 
+## 1.10.4 / 2019-08-07
+
+### Security
+
+#### Address CVE-2019-5477 (#1915)
+
+A command injection vulnerability in Nokogiri v1.10.3 and earlier allows commands to be executed in a subprocess by Ruby's `Kernel.open` method. Processes are vulnerable only if the undocumented method `Nokogiri::CSS::Tokenizer#load_file` is being passed untrusted user input.
+
+This vulnerability appears in code generated by the Rexical gem versions v1.0.6 and earlier. Rexical is used by Nokogiri to generate lexical scanner code for parsing CSS queries. The underlying vulnerability was addressed in Rexical v1.0.7 and Nokogiri upgraded to this version of Rexical in Nokogiri v1.10.4.
+
+This CVE's public notice is https://github.com/sparklemotion/nokogiri/issues/1915
+
+
 ## 1.10.3 / 2019-04-22
 
 ### Security Notes

diff --git a/Gemfile b/Gemfile
@@ -17,6 +17,7 @@ gem "rake", "~>12.0", :group => [:development, :test]
 gem "rake-compiler", "~>1.0.3", :group => [:development, :test]
 gem "rake-compiler-dock", "~>0.7.0", :group => [:development, :test]
 gem "rexical", "~>1.0.5", :group => [:development, :test]
+gem "rubocop", "~>0.73", :group => [:development, :test]
 gem "simplecov", "~>0.16", :group => [:development, :test]
 gem "rdoc", ">=4.0", "<7", :group => [:development, :test]
 gem "hoe", "~>3.17", :group => [:development, :test]

diff --git a/Rakefile b/Rakefile
@@ -143,6 +143,7 @@ HOE = Hoe.spec 'nokogiri' do
     ["rake-compiler",      "~> 1.0.3"],
     ["rake-compiler-dock", "~> 0.7.0"],
     ["rexical",            "~> 1.0.5"],
+    ["rubocop",            "~> 0.73"],
     ["simplecov",          "~> 0.16"],
   ]
 
@@ -275,6 +276,11 @@ task :java_debug do
 end
 Rake::Task[:test].prerequisites << :java_debug
 
+task :rubocop_security do
+  sh "rubocop lib --only Security"
+end
+Rake::Task[:test].prerequisites << :rubocop_security
+
 if Hoe.plugins.include?(:debugging)
   ['valgrind', 'valgrind:mem', 'valgrind:mem0'].each do |task_name|
     Rake::Task["test:#{task_name}"].prerequisites << :compile

diff --git a/lib/nokogiri/css/tokenizer.rb b/lib/nokogiri/css/tokenizer.rb
@@ -1,151 +1,152 @@
 #--
 # DO NOT MODIFY!!!!
-# This file is automatically generated by rex 1.0.5
+# This file is automatically generated by rex 1.0.7
 # from lexical definition file "lib/nokogiri/css/tokenizer.rex".
 #++
 
 module Nokogiri
 module CSS
 class Tokenizer # :nodoc:
-  require 'strscan'
+      require 'strscan'
 
-  class ScanError < StandardError ; end
+      class ScanError < StandardError ; end
 
-  attr_reader   :lineno
-  attr_reader   :filename
-  attr_accessor :state
+      attr_reader   :lineno
+      attr_reader   :filename
+      attr_accessor :state
 
-  def scan_setup(str)
-    @ss = StringScanner.new(str)
-    @lineno =  1
-    @state  = nil
-  end
+      def scan_setup(str)
+        @ss = StringScanner.new(str)
+        @lineno =  1
+        @state  = nil
+      end
 
-  def action
-    yield
-  end
+      def action
+        yield
+      end
 
-  def scan_str(str)
-    scan_setup(str)
-    do_parse
-  end
-  alias :scan :scan_str
+      def scan_str(str)
+        scan_setup(str)
+        do_parse
+      end
+      alias :scan :scan_str
 
-  def load_file( filename )
-    @filename = filename
-    open(filename, "r") do |f|
-      scan_setup(f.read)
-    end
-  end
+      def load_file( filename )
+        @filename = filename
+        File.open(filename, "r") do |f|
+          scan_setup(f.read)
+        end
+      end
 
-  def scan_file( filename )
-    load_file(filename)
-    do_parse
-  end
+      def scan_file( filename )
+        load_file(filename)
+        do_parse
+      end
 
 
-  def next_token
-    return if @ss.eos?
-
-    # skips empty actions
-    until token = _next_token or @ss.eos?; end
-    token
-  end
+        def next_token
+          return if @ss.eos?
 
-  def _next_token
-    text = @ss.peek(1)
-    @lineno  +=  1  if text == "\n"
-    token = case @state
-    when nil
-      case
-      when (text = @ss.scan(/has\([\s]*/))
-         action { [:HAS, text] }
+          # skips empty actions
+          until token = _next_token or @ss.eos?; end
+          token
+        end
 
-      when (text = @ss.scan(/[-@]?([_A-Za-z]|[^\0-\177]|\\[0-9A-Fa-f]{1,6}(\r\n|[\s])?|\\[^\n\r\f0-9A-Fa-f])([_A-Za-z0-9-]|[^\0-\177]|\\[0-9A-Fa-f]{1,6}(\r\n|[\s])?|\\[^\n\r\f0-9A-Fa-f])*\([\s]*/))
-         action { [:FUNCTION, text] }
+        def _next_token
+          text = @ss.peek(1)
+          @lineno  +=  1  if text == "\n"
+          token = case @state
+            when nil
+          case
+                  when (text = @ss.scan(/has\([\s]*/))
+                     action { [:HAS, text] }
 
-      when (text = @ss.scan(/[-@]?([_A-Za-z]|[^\0-\177]|\\[0-9A-Fa-f]{1,6}(\r\n|[\s])?|\\[^\n\r\f0-9A-Fa-f])([_A-Za-z0-9-]|[^\0-\177]|\\[0-9A-Fa-f]{1,6}(\r\n|[\s])?|\\[^\n\r\f0-9A-Fa-f])*/))
-         action { [:IDENT, text] }
+                  when (text = @ss.scan(/[-@]?([_A-Za-z]|[^\0-\177]|\\[0-9A-Fa-f]{1,6}(\r\n|[\s])?|\\[^\n\r\f0-9A-Fa-f])([_A-Za-z0-9-]|[^\0-\177]|\\[0-9A-Fa-f]{1,6}(\r\n|[\s])?|\\[^\n\r\f0-9A-Fa-f])*\([\s]*/))
+                     action { [:FUNCTION, text] }
 
-      when (text = @ss.scan(/\#([_A-Za-z0-9-]|[^\0-\177]|\\[0-9A-Fa-f]{1,6}(\r\n|[\s])?|\\[^\n\r\f0-9A-Fa-f])+/))
-         action { [:HASH, text] }
+                  when (text = @ss.scan(/[-@]?([_A-Za-z]|[^\0-\177]|\\[0-9A-Fa-f]{1,6}(\r\n|[\s])?|\\[^\n\r\f0-9A-Fa-f])([_A-Za-z0-9-]|[^\0-\177]|\\[0-9A-Fa-f]{1,6}(\r\n|[\s])?|\\[^\n\r\f0-9A-Fa-f])*/))
+                     action { [:IDENT, text] }
 
-      when (text = @ss.scan(/[\s]*~=[\s]*/))
-         action { [:INCLUDES, text] }
+                  when (text = @ss.scan(/\#([_A-Za-z0-9-]|[^\0-\177]|\\[0-9A-Fa-f]{1,6}(\r\n|[\s])?|\\[^\n\r\f0-9A-Fa-f])+/))
+                     action { [:HASH, text] }
 
-      when (text = @ss.scan(/[\s]*\|=[\s]*/))
-         action { [:DASHMATCH, text] }
+                  when (text = @ss.scan(/[\s]*~=[\s]*/))
+                     action { [:INCLUDES, text] }
 
-      when (text = @ss.scan(/[\s]*\^=[\s]*/))
-         action { [:PREFIXMATCH, text] }
+                  when (text = @ss.scan(/[\s]*\|=[\s]*/))
+                     action { [:DASHMATCH, text] }
 
-      when (text = @ss.scan(/[\s]*\$=[\s]*/))
-         action { [:SUFFIXMATCH, text] }
+                  when (text = @ss.scan(/[\s]*\^=[\s]*/))
+                     action { [:PREFIXMATCH, text] }
 
-      when (text = @ss.scan(/[\s]*\*=[\s]*/))
-         action { [:SUBSTRINGMATCH, text] }
+                  when (text = @ss.scan(/[\s]*\$=[\s]*/))
+                     action { [:SUFFIXMATCH, text] }
 
-      when (text = @ss.scan(/[\s]*!=[\s]*/))
-         action { [:NOT_EQUAL, text] }
+                  when (text = @ss.scan(/[\s]*\*=[\s]*/))
+                     action { [:SUBSTRINGMATCH, text] }
 
-      when (text = @ss.scan(/[\s]*=[\s]*/))
-         action { [:EQUAL, text] }
+                  when (text = @ss.scan(/[\s]*!=[\s]*/))
+                     action { [:NOT_EQUAL, text] }
 
-      when (text = @ss.scan(/[\s]*\)/))
-         action { [:RPAREN, text] }
+                  when (text = @ss.scan(/[\s]*=[\s]*/))
+                     action { [:EQUAL, text] }
 
-      when (text = @ss.scan(/\[[\s]*/))
-         action { [:LSQUARE, text] }
+                  when (text = @ss.scan(/[\s]*\)/))
+                     action { [:RPAREN, text] }
 
-      when (text = @ss.scan(/[\s]*\]/))
-         action { [:RSQUARE, text] }
+                  when (text = @ss.scan(/\[[\s]*/))
+                     action { [:LSQUARE, text] }
 
-      when (text = @ss.scan(/[\s]*\+[\s]*/))
-         action { [:PLUS, text] }
+                  when (text = @ss.scan(/[\s]*\]/))
+                     action { [:RSQUARE, text] }
 
-      when (text = @ss.scan(/[\s]*>[\s]*/))
-         action { [:GREATER, text] }
+                  when (text = @ss.scan(/[\s]*\+[\s]*/))
+                     action { [:PLUS, text] }
 
-      when (text = @ss.scan(/[\s]*,[\s]*/))
-         action { [:COMMA, text] }
+                  when (text = @ss.scan(/[\s]*>[\s]*/))
+                     action { [:GREATER, text] }
 
-      when (text = @ss.scan(/[\s]*~[\s]*/))
-         action { [:TILDE, text] }
+                  when (text = @ss.scan(/[\s]*,[\s]*/))
+                     action { [:COMMA, text] }
 
-      when (text = @ss.scan(/\:not\([\s]*/))
-         action { [:NOT, text] }
+                  when (text = @ss.scan(/[\s]*~[\s]*/))
+                     action { [:TILDE, text] }
 
-      when (text = @ss.scan(/-?([0-9]+|[0-9]*\.[0-9]+)/))
-         action { [:NUMBER, text] }
+                  when (text = @ss.scan(/\:not\([\s]*/))
+                     action { [:NOT, text] }
 
-      when (text = @ss.scan(/[\s]*\/\/[\s]*/))
-         action { [:DOUBLESLASH, text] }
+                  when (text = @ss.scan(/-?([0-9]+|[0-9]*\.[0-9]+)/))
+                     action { [:NUMBER, text] }
 
-      when (text = @ss.scan(/[\s]*\/[\s]*/))
-         action { [:SLASH, text] }
+                  when (text = @ss.scan(/[\s]*\/\/[\s]*/))
+                     action { [:DOUBLESLASH, text] }
 
-      when (text = @ss.scan(/U\+[0-9a-f?]{1,6}(-[0-9a-f]{1,6})?/))
-         action {[:UNICODE_RANGE, text] }
+                  when (text = @ss.scan(/[\s]*\/[\s]*/))
+                     action { [:SLASH, text] }
 
-      when (text = @ss.scan(/[\s]+/))
-         action { [:S, text] }
+                  when (text = @ss.scan(/U\+[0-9a-f?]{1,6}(-[0-9a-f]{1,6})?/))
+                     action {[:UNICODE_RANGE, text] }
 
-      when (text = @ss.scan(/"([^\n\r\f"]|\n|\r\n|\r|\f|[^\0-\177]|\\[0-9A-Fa-f]{1,6}(\r\n|[\s])?|\\[^\n\r\f0-9A-Fa-f])*(?<!\\)(?:\\{2})*"|'([^\n\r\f']|\n|\r\n|\r|\f|[^\0-\177]|\\[0-9A-Fa-f]{1,6}(\r\n|[\s])?|\\[^\n\r\f0-9A-Fa-f])*(?<!\\)(?:\\{2})*'/))
-         action { [:STRING, text] }
+                  when (text = @ss.scan(/[\s]+/))
+                     action { [:S, text] }
 
-      when (text = @ss.scan(/./))
-         action { [text, text] }
+                  when (text = @ss.scan(/"([^\n\r\f"]|\n|\r\n|\r|\f|[^\0-\177]|\\[0-9A-Fa-f]{1,6}(\r\n|[\s])?|\\[^\n\r\f0-9A-Fa-f])*(?<!\\)(?:\\{2})*"|'([^\n\r\f']|\n|\r\n|\r|\f|[^\0-\177]|\\[0-9A-Fa-f]{1,6}(\r\n|[\s])?|\\[^\n\r\f0-9A-Fa-f])*(?<!\\)(?:\\{2})*'/))
+                     action { [:STRING, text] }
 
-      else
-        text = @ss.string[@ss.pos .. -1]
-        raise  ScanError, "can not match: '" + text + "'"
-      end  # if
+                  when (text = @ss.scan(/./))
+                     action { [text, text] }
 
-    else
-      raise  ScanError, "undefined state: '" + state.to_s + "'"
-    end  # case state
-    token
-  end  # def _next_token
+
+          else
+            text = @ss.string[@ss.pos .. -1]
+            raise  ScanError, "can not match: '" + text + "'"
+          end  # if
+
+        else
+          raise  ScanError, "undefined state: '" + state.to_s + "'"
+        end  # case state
+          token
+        end  # def _next_token
 
 end # class
 end