18

I'm building a daemon that will help me manage my server(s). Webmin works fine, as does just opening a shell to the server, but I'd prefer to be able to control server operations from a UI I design, and also expose some functionality to end users.

The daemon will pick up actions from a queue and execute them. However, since I'll be accepting input from users, I want to make sure they're not permitted to inject something dangerous into a privileged shell command.

Here's a fragment that exemplifies my problem:

def perform
  system "usermod -p #{@options['shadow']} #{@options['username']}"
end

A gist that explains more: https://gist.github.com/773292

I'm not positive if typical escaping and sanitizing of inputs is enough for this case, and being a designer, I don't have a ton of security-related experience. I know that this is something that should probably be obvious to me, but its not!

How can I ensure that the web application that will create and serialize the actions can't pass dangerous text into the privileged process that receives the actions?

Thanks for the help
arb

arbales
  • 5,466
  • 4
  • 33
  • 40

6 Answers6

21

It doesn't look like you need a shell for what you're doing. See the documentation for system here: http://ruby-doc.org/core/classes/Kernel.html#M001441

You should use the second form of system. Your example above would become:

system 'usermod', '-p', @options['shadow'], @options['username']

A nicer (IMO) way to write this is:

system *%W(usermod -p #{@options['shadow']} #{@options['username']})

The arguments this way are passed directly into the execve call, so you don't have to worry about sneaky shell tricks.

cam
  • 14,192
  • 1
  • 44
  • 29
  • so, let's say if I exposed this class, completely unfiltered (I won't) to an end user, and they provided a shadow hash and then "username; rm -rf /" as the username — this wouldn't have the effect of obliterating `/` – arbales Jan 11 '11 at 04:16
  • 4
    Correct. The arguments are passed directly to the executed program. You can verify this by yourself. Try running `ruby -e 'system *W(ls -l foo; rm -rf /)'` – cam Jan 11 '11 at 04:41
  • ah, well great. It does make perfect sense. I think I have this thought that ensuring an application is secure must naturally be harder than it is, harder than simple steps and facts, as if there exists a dangerous edge case for everything. This is likely because I've never read / learned very much about it. – arbales Jan 11 '11 at 05:00
  • this also has the added benefit of using system calls, instead of making the calls through bash and then to the system (saves a little overhead on top of the sanitization) – Stuart Nelson May 09 '13 at 18:43
  • The example given by above should read `ruby -e 'system *%w(ls -l foo; rm -rf /)'` instead. – Daniel Le Nov 03 '21 at 13:04
17

If you need not just the exit status but also the result you probably want to use Open3.popen3:

require 'open3'
stdin, stdout, stderr = Open3.popen3('usermod', '-p', @options['shadow'], @options['username'])
stdout.gets
sterr.gets

More information here: Getting output of system() calls in Ruby

Community
  • 1
  • 1
Simon Hürlimann
  • 4,181
  • 3
  • 20
  • 23
2

I'd suggest looking into the 'shellwords' module. This script:

require 'shellwords'
parts = ['echo', "'hello world'; !%& some stuff", 'and another argument']
command = Shellwords.shelljoin( parts )
puts command
output = `#{ command }`
puts output

outputs the escaped text and the expected output:

echo \'hello\ world\'\;\ \!\%\&\ some\ stuff and\ another\ argument
'hello world'; !%& some stuff and another argument
Dan
  • 141
  • 1
  • 3
1

This is an old question, but since it's pretty much the only real answer you'll find when googling I thought I'd add a caveat. The multi argument version of system seems reasonably safe on Linux, but it is NOT on Windows.

Try system "dir", "&", "echo", "hi!" on a Windows system. Both dir and echo will be run. Echo could of course just as well be something far less innocuous.

user2261892
  • 188
  • 7
0

I know this is an old thread, but there is another option that was lightly touched on by Simon Hürlimann.

There is not a lot of information about this topic and I think this might help others in need.

For this example we'll use Open3 which gives you the ability to run commands synchronously or asynchronously, and provides stdout, stderr, exit codes, and PID.

Open3 grants you access to stdout, stderr, exit codes and a thread to wait for the child process when running another program. You can specify various attributes, redirections, current directory, etc., of the program in the same way as for Process.spawn. (Source: Open3 Docs)

I chose to format the output as a CommandStatus object. This contains our stdout, stderr, pid (Of the worker thread) and exitstatus.

class Command
  require 'open3'

  class CommandStatus
    @stdout     = nil
    @stderr     = nil
    @pid        = nil
    @exitstatus = nil

    def initialize(stdout, stderr, process)
      @stdout     = stdout
      @stderr     = stderr
      @pid        = process.pid
      @exitstatus = process.exitstatus
    end

    def stdout
      @stdout
    end

    def stderr
      @stderr
    end

    def exit_status
      @exitstatus
    end

    def pid
      @pid
    end
  end

  def self.execute(command)
    command_stdout = nil
    command_stderr = nil
    process = Open3.popen3(ENV, command + ';') do |stdin, stdout, stderr, thread|
      stdin.close
      stdout_buffer   = stdout.read
      stderr_buffer   = stderr.read
      command_stdout  = stdout_buffer if stdout_buffer.length > 0
      command_stderr  = stderr_buffer if stderr_buffer.length > 0
      thread.value # Wait for Process::Status object to be returned
    end
    return CommandStatus.new(command_stdout, command_stderr, process)
  end
end


cmd = Command::execute("echo {1..10}")

puts "STDOUT: #{cmd.stdout}"
puts "STDERR: #{cmd.stderr}"
puts "EXIT: #{cmd.exit_status}"

While reading the STDOUT/ERR buffers, I use command_stdout = stdout_buffer if stdout_buffer.length > 0 to control whether the command_stdout variable is assigned or not. You should pass nil instead of "" when no data is present. It's more clear when handing data later on.

You probably noticed me using command + ';'. The reason for this is based on the documentation from Kernel.exec (Which is what popen3 uses):

If the string from the first form (exec("command")) follows these simple rules:

  • no meta characters
  • no shell reserved word and no special built-in
  • Ruby invokes the command directly without shell

You can force shell invocation by adding ";" to the string (because ";" is a meta character)

This simply prevents a Ruby from throwing a 'spawn': No such file or directory error if you pass a malformed command. Instead it will pass it straight to the kernel where the error will be resolved gracefully and appear as STDERR instead of an uncaught exception.

Jakir00
  • 2,023
  • 4
  • 20
  • 32
0

Modern, secure and simple solution (popen will escape arguments for you):

IO.popen(['usermod', '-p', @options['shadow'], @options['username']]).read

(#read will close the IO before returning)

Paweł Gościcki
  • 9,066
  • 5
  • 70
  • 81