fixing shell injection issue in options

bin/wkhtmltopdf-proxy

@@ -16,6 +16,4 @@ if executable.empty?
 
 end
 
-arguments = $*.map { |arg| arg.include?(' ') ? %Q("#{arg}") : arg }
-
-system(executable + " " + arguments.join(" "))
+system(executable, *($*))

lib/pdfkit/pdfkit.rb

@@ -45,13 +45,14 @@ def command
     end
 
     args << '-' # Read PDF from stdout
-    args.join(' ')
+    args
   end
 
   def to_pdf
     append_stylesheets
 
-    pdf = IO.popen(command, "w+")
+    pdf = Kernel.open('|-', "w+")
+    exec(*command) if pdf.nil?
     pdf.puts(@source.to_s) if @source.html?
     pdf.close_write
     result = pdf.gets(nil)
@@ -103,8 +104,6 @@ def normalize_value(value)
       case value
       when TrueClass
         nil
-      when String
-        value.match(/\s/) ? "\"#{value}\"" : value
       else
         value
       end

spec/pdfkit_spec.rb

@@ -43,9 +43,9 @@
   context "command" do
     it "should contstruct the correct command" do
       pdfkit = PDFKit.new('html', :page_size => 'Letter', :toc_l1_font_size => 12)
-      pdfkit.command.should include('wkhtmltopdf')
-      pdfkit.command.should include('--page-size Letter')
-      pdfkit.command.should include('--toc-l1-font-size 12')
+      pdfkit.command[0].should include('wkhtmltopdf')
+      pdfkit.command[pdfkit.command.index('--page-size') + 1].should == 'Letter'
+      pdfkit.command[pdfkit.command.index('--toc-l1-font-size') + 1].should == 12
     end
 
     it "will not include default options it is told to omit" do
@@ -57,23 +57,23 @@
 
     it "should encapsulate string arguments in quotes" do
       pdfkit = PDFKit.new('html', :header_center => "foo [page]")
-      pdfkit.command.should include('--header-center "foo [page]"')
+      pdfkit.command[pdfkit.command.index('--header-center') + 1].should == 'foo [page]'
     end
 
     it "read the source from stdin if it is html" do
       pdfkit = PDFKit.new('html')
-      pdfkit.command.should match(/ - -$/)
+      pdfkit.command[-2..-1].should == ['-', '-']
     end
 
     it "specify the URL to the source if it is a url" do
       pdfkit = PDFKit.new('http://google.com')
-      pdfkit.command.should match(/ http:\/\/google\.com -$/)
+      pdfkit.command[-2..-1].should == ['http://google.com', '-']
     end
 
     it "should specify the path to the source if it is a file" do
       file_path = File.join(SPEC_ROOT,'fixtures','example.html')
       pdfkit = PDFKit.new(File.new(file_path))
-      pdfkit.command.should match(/ #{file_path} -$/)
+      pdfkit.command[-2..-1].should == [file_path, '-']
     end
   end
 
@@ -127,4 +127,21 @@
     end
   end
 
+  context "security" do
+    before do
+      @test_path = File.join(SPEC_ROOT,'fixtures','security-oops')
+      File.delete(@test_path) if File.exist?(@test_path)
+    end
+
+    after do
+      File.delete(@test_path) if File.exist?(@test_path)
+    end
+
+    it "should not allow shell injection in options" do
+      pdfkit = PDFKit.new('html', :header_center => "a title\"; touch #{@test_path} #")
+      pdfkit.to_pdf
+      File.exist?(@test_path).should be_false
+    end
+  end
+
 end