thoughtbot / cocaine

A small library for doing (command) lines.
https://robots.thoughtbot.com
Other
785 stars 55 forks source link

Shellquotes not working #93

Closed martijncasteel closed 7 years ago

martijncasteel commented 8 years ago

Shell quotes not working

I'm not sure why, and it cost me a few days a year ago . But when trying to use paperclip and a processor for ghostscript I have to overwrite the shellquotes. I have found this fix on the internet and I believe it is used quite sometime. This week I updated a lot of gems and I was hoping that I good remove this dirty fix..

  # receives a pdf to convert to png
  has_attached_file :poster,
    :styles => { :thumb => ['180', :png], :medium => ['x720', :png] },
    :processors => [:ghostscript, :thumbnail],
    :validate_media_type => false,
    :convert_options => { :all => '-colorspace CMYK -flatten -quality 100 -density 8' }
module Paperclip
  class Ghostscript < Processor

    attr_accessor :current_geometry, :target_geometry, :format, :whiny, :convert_options, :source_file_options

    def initialize file, options = {}, attachment = nil
      super
      @file                = file
      @format              = options[:format]

      @current_format      = File.extname(@file.path)
      @basename            = File.basename(@file.path, @current_format)
    end

    def make
      src = @file
      dst = Tempfile.new([@basename, @format ? ".#{@format}" : ''])
      dst.binmode

      begin
        parameters = []
        parameters << '-dNOPAUSE -dBATCH -sDEVICE=pngalpha -r144 -dUseCIEColor'
        parameters << '-sOutputFile=:dest'
        parameters << ':source'

        parameters = parameters.flatten.compact.join(' ').strip.squeeze(' ')

        success = Paperclip.run('gs', parameters, :source => "#{File.expand_path(src.path)}", :dest => File.expand_path(dst.path))
      rescue PaperclipCommandLineError => e
        raise PaperclipError, "There was an error processing the thumbnail for #{@basename}" if @whiny
      end
      dst
    end
  end
end

But unfortunately I get the dreaded NotIdentifiedByImageMagick error. But I scouerd the internet and found this which fixes my problem;

Cocaine::CommandLine.module_eval do

  def run(interpolations = {})
    @exit_status = nil
    begin
      full_command = command(interpolations)
      log("#{colored("Command")} :: #{full_command}")
      @output = execute(full_command)
    rescue Errno::ENOENT => e
      raise Cocaine::CommandNotFoundError, e.message
    ensure
      @exit_status = $?.respond_to?(:exitstatus) ? $?.exitstatus : 0
    end

    if @exit_status == 127
      raise Cocaine::CommandNotFoundError
    end

    unless @expected_outcodes.include?(@exit_status)
      message = [
        "Command '#{full_command}' returned #{@exit_status}. Expected #{@expected_outcodes.join(", ")}",
        "Here is the command output: STDOUT:\n", command_output,
        "\nSTDERR:\n", command_error_output
      ].join("\n")
      raise Cocaine::ExitStatusError, message
    end

    command_output
  end

  private
  def interpolate(pattern, interpolations)
    interpolations = stringify_keys(interpolations)

    pattern.gsub(/:\{?(\w+)\b\}?/) do |match|

      key = match.tr(":{}", "")

      if interpolations.key?(key)
        "'#{interpolations[key]}'"
      else
        match
      end
    end
  end
end

Well the upperpart is just that cocaine uses my interpolate but why is this still a problem using paperclip?

jyurek commented 7 years ago

I realize this is a year old, but in the interest of figuring this out I'd need to know what a command looked like before and after you fix.

FWIW, your fix is not secure. It allows for shell command injection with a specially crafted file name.

jyurek commented 7 years ago

This is now well over a year old and has had plenty of time for feedback, so I'm going to close it. If it's still a problem, please resubmit.