[DRE-commits] [gem2deb] 02/03: Don't interpolate arguments for shell commands

Antonio Terceiro terceiro at moszumanska.debian.org
Thu Nov 21 18:00:02 UTC 2013


This is an automated email from the git hooks/post-receive script.

terceiro pushed a commit to branch master
in repository gem2deb.

commit 1b6ae64445c574f50cde208f6943be166a31b7f2
Author: Antonio Terceiro <terceiro at debian.org>
Date:   Wed Nov 20 15:34:22 2013 -0300

    Don't interpolate arguments for shell commands
    
    This fixes handling of files with weird characters in their names such
    as ")" and whitespace in general, and improves security against
    maliciously-crafted filenames which could inject unwanted shell
    commands in the system that is building a package with gem2deb.
    
    Git-Dch: Full
    Closes: #729981
---
 bin/gem2deb                       |    2 +-
 bin/gem2tgz                       |    2 +-
 lib/gem2deb.rb                    |   14 ++++++++++----
 lib/gem2deb/dh_make_ruby.rb       |    8 ++++----
 lib/gem2deb/dh_ruby.rb            |    3 +--
 lib/gem2deb/gem2tgz.rb            |    6 +++---
 lib/gem2deb/installer.rb          |    8 ++++----
 lib/gem2deb/setup_rb_installer.rb |    8 ++++----
 test/unit/secure_code_test.rb     |   13 +++++++++++++
 9 files changed, 41 insertions(+), 23 deletions(-)

diff --git a/bin/gem2deb b/bin/gem2deb
index 4aa8952..02d1aa3 100755
--- a/bin/gem2deb
+++ b/bin/gem2deb
@@ -74,7 +74,7 @@ gemfile = ARGV[0]
 # Download gem if not available locally
 if not File::exists?(gemfile) and gemfile !~ /.gem$/
   puts "#{gemfile} doesn't seem to exist. Let's try to download it with 'gem fetch #{ARGV[0]}'"
-  run("gem fetch #{gemfile}")
+  run("gem", "fetch", gemfile)
   gemfile = Dir::glob("#{gemfile}-*.gem")[0]
 end
 
diff --git a/bin/gem2tgz b/bin/gem2tgz
index f063539..bc49660 100755
--- a/bin/gem2tgz
+++ b/bin/gem2tgz
@@ -41,7 +41,7 @@ gemfile = ARGV[0]
 # Download gem if not available locally
 if not File::exists?(gemfile) and gemfile !~ /.gem$/
   puts "#{gemfile} doesn't seem to exist. Let's try to download it with 'gem fetch #{ARGV[0]}'"
-  run("gem fetch #{gemfile}")
+  run("gem", "fetch", gemfile)
   gemfile = Dir::glob("#{gemfile}-*.gem")[0]
 end
 
diff --git a/lib/gem2deb.rb b/lib/gem2deb.rb
index f675144..50b5de3 100644
--- a/lib/gem2deb.rb
+++ b/lib/gem2deb.rb
@@ -47,13 +47,19 @@ module Gem2Deb
 
   LIBDIR = File.expand_path(File.dirname(__FILE__))
 
-  def run(cmd)
-    puts(cmd) if $VERBOSE
-    system(cmd)
+  def run(*argv)
+    puts(_format_cmdline(argv)) if $VERBOSE
+    system(*argv)
     if $?.exitstatus != 0
-      raise Gem2Deb::CommandFailed, "[#{cmd} failed!]"
+      raise Gem2Deb::CommandFailed, _format_cmdline(argv)
     end
   end
+
+  private
+
+  def _format_cmdline(argv)
+    argv.map { |a| a =~ /\s/ && a.inspect || a }.join(' ')
+  end
 end
 
 require 'gem2deb/version'
diff --git a/lib/gem2deb/dh_make_ruby.rb b/lib/gem2deb/dh_make_ruby.rb
index a9be689..7ceb53c 100644
--- a/lib/gem2deb/dh_make_ruby.rb
+++ b/lib/gem2deb/dh_make_ruby.rb
@@ -167,18 +167,18 @@ module Gem2Deb
       dpkg_buildpackage_opts << '-d' unless check_build_deps
 
       Dir.chdir(source_dirname) do
-        run("dpkg-buildpackage -us -uc #{dpkg_buildpackage_opts.join(' ')}")
+        run('dpkg-buildpackage', '-us', '-uc', *dpkg_buildpackage_opts)
       end
     end
 
     def create_orig_tarball
       if source_package_name != orig_tarball_name && !File.exists?(orig_tarball_name)
-        run "ln -s #{source_tarball_name} #{orig_tarball_name}"
+        run('ln', '-s', source_tarball_name, orig_tarball_name)
       end
     end
 
     def extract
-      run("tar xzf #{orig_tarball_name}")
+      run('tar', 'xzf', orig_tarball_name)
       if !File.directory?(gem_dirname)
         raise "Extracting did not create #{gem_dirname} directory."
       end
@@ -190,7 +190,7 @@ module Gem2Deb
     def create_debian_boilerplates
       FileUtils.mkdir_p('debian')
       unless File.exists?('debian/changelog')
-        run "dch --create --empty --package #{source_package_name} --newversion #{gem_version}-1 'Initial release (Closes: #nnnn)'"
+        run('dch', '--create', '--empty', '--package', source_package_name, '--newversion', "#{gem_version}-1", 'Initial release (Closes: #nnnn)')
       end
       templates.each do |template|
         FileUtils.mkdir_p(template.directory)
diff --git a/lib/gem2deb/dh_ruby.rb b/lib/gem2deb/dh_ruby.rb
index 604ebf6..8dc2aaa 100644
--- a/lib/gem2deb/dh_ruby.rb
+++ b/lib/gem2deb/dh_ruby.rb
@@ -135,8 +135,7 @@ module Gem2Deb
         return
       end
 
-      cmd = "#{SUPPORTED_RUBY_VERSIONS[rubyver]} -I#{LIBDIR} #{TEST_RUNNER}"
-      run(cmd)
+      run(SUPPORTED_RUBY_VERSIONS[rubyver], '-I' + LIBDIR, TEST_RUNNER)
 
       if $?.exitstatus != 0
         handle_test_failure(rubyver)
diff --git a/lib/gem2deb/gem2tgz.rb b/lib/gem2deb/gem2tgz.rb
index 22ea685..f40cd8c 100644
--- a/lib/gem2deb/gem2tgz.rb
+++ b/lib/gem2deb/gem2tgz.rb
@@ -73,7 +73,7 @@ module Gem2Deb
 
     def extract_gem_contents
       Dir.chdir(@target_dir) do
-        run "tar xfm #{gem_full_path}"
+        run('tar', 'xfm', gem_full_path)
         run 'tar xzfm data.tar.gz'
         FileUtils.rm_f('data.tar.gz')
         run "zcat metadata.gz > metadata.yml"
@@ -83,13 +83,13 @@ module Gem2Deb
 
     def extract_tgz_contents
       Dir.chdir(@target_dir) do
-        run "tar xfm #{gem_full_path} --strip 1"
+        run('tar', 'xfm', gem_full_path, '--strip', '1')
       end
     end
 
     def create_resulting_tarball
       Dir.chdir(@tmp_dir) do
-        run "tar czf #{@tarball_full_path} #{@target_dirname}"
+        run('tar', 'czf', @tarball_full_path, @target_dirname)
       end
     end
 
diff --git a/lib/gem2deb/installer.rb b/lib/gem2deb/installer.rb
index 5531818..0324a96 100644
--- a/lib/gem2deb/installer.rb
+++ b/lib/gem2deb/installer.rb
@@ -33,7 +33,7 @@ module Gem2Deb
       if metadata.has_native_extensions?
         ruby_versions.each do |rubyver|
           puts "Building extension for #{rubyver} ..." if verbose
-          run("#{SUPPORTED_RUBY_VERSIONS[rubyver]} -I#{LIBDIR} #{EXTENSION_BUILDER} #{root} #{destdir_base}")
+          run(SUPPORTED_RUBY_VERSIONS[rubyver], "-I#{LIBDIR}", EXTENSION_BUILDER, root, destdir_base)
 
           # Remove duplicate files installed by rubygems in the arch dir
           # This is a hack to workaround a problem in rubygems
@@ -175,7 +175,7 @@ module Gem2Deb
 
 
     def install_files(src, dest, mode)
-      run "install -d #{dest}"
+      run("install", "-d", dest)
       files_to_install = Dir.chdir(src) do
         Dir.glob('**/*').reject do |file|
           filename = File.basename(file)
@@ -185,7 +185,7 @@ module Gem2Deb
       files_to_install.each do |file|
         from = File.join(src, file)
         to = File.join(dest, file)
-        run "install -D -m#{mode} #{from} #{to}"
+        run("install", "-D", "-m#{mode}", from, to)
       end
     end
 
@@ -259,7 +259,7 @@ module Gem2Deb
     def install_changelog
       changelog = Dir.glob(File.join(root, 'CHANGELOG*')).first
       if changelog
-        run("dh_installchangelogs -p#{binary_package} #{changelog} upstream")
+        run("dh_installchangelogs", "-p#{binary_package}", changelog, "upstream")
       end
     end
 
diff --git a/lib/gem2deb/setup_rb_installer.rb b/lib/gem2deb/setup_rb_installer.rb
index 65569d3..a831783 100644
--- a/lib/gem2deb/setup_rb_installer.rb
+++ b/lib/gem2deb/setup_rb_installer.rb
@@ -28,16 +28,16 @@ module Gem2Deb
         prefix = destdir(:prefix, rubyver)
 
         # First configure
-        run("#{ruby} setup.rb config --prefix=#{prefix} --bindir=#{bindir} --siteruby=#{siteruby} --siterubyver=#{siteruby} --siterubyverarch=#{archdir}")
+        run(ruby, 'setup.rb', 'config', "--prefix=#{prefix}", "--bindir=#{bindir}", "--siteruby=#{siteruby}", "--siterubyver=#{siteruby}", "--siterubyverarch=#{archdir}")
 
         # Then setup
-        run("#{ruby} setup.rb setup")
+        run(ruby, 'setup.rb', 'setup')
 
         # Then install
-        run("#{ruby} setup.rb install")
+        run(ruby, 'setup.rb', 'install')
 
         # Then clean
-        run("#{ruby} setup.rb distclean")
+        run(ruby, 'setup.rb', 'distclean')
 
       end
     end
diff --git a/test/sample/install_files/lib/mylib/foo(bar).rb b/test/sample/install_files/lib/mylib/foo(bar).rb
new file mode 100644
index 0000000..e69de29
diff --git a/test/unit/secure_code_test.rb b/test/unit/secure_code_test.rb
new file mode 100644
index 0000000..78cca56
--- /dev/null
+++ b/test/unit/secure_code_test.rb
@@ -0,0 +1,13 @@
+require 'test_helper'
+
+class SecureCodeTest < Gem2DebTestCase
+
+  should 'not interpolate variables into shell commands' do
+    insecure_code = `grep -rl '\\(system\\|run\\)[( ][^,]*\#{' lib/ bin/`.split
+    unless insecure_code.empty?
+      fail "files containing insecure code: \n\t" + insecure_code.join("\n\t")
+    end
+  end
+
+
+end

-- 
Alioth's /usr/local/bin/git-commit-notice on /srv/git.debian.org/git/pkg-ruby-extras/gem2deb.git



More information about the Pkg-ruby-extras-commits mailing list