Permalink
Browse files

security: prevent remote command injection in content_type

  • Loading branch information...
1 parent 8b46cdb commit ca0302e0c716682d2de22e9136400c704cc93e42 @gauteh gauteh committed Oct 28, 2013
Showing with 7 additions and 2 deletions.
  1. +7 −2 lib/sup/message_chunks.rb
@@ -113,6 +113,11 @@ class Attachment
def initialize content_type, filename, encoded_content, sibling_types
@content_type = content_type.downcase
+ if Shellwords.escape(@content_type) != @content_type
+ warn "content_type #{@content_type} is not safe, changed to application/octet-stream"
+ @content_type = 'application/octet-stream'
+ end
+
@filename = filename
@quotable = false # changed to true if we can parse it through the
# mime-decode hook, or if it's plain text
@@ -129,7 +134,7 @@ def initialize content_type, filename, encoded_content, sibling_types
else
## please see note in write_to_disk on important usage
## of quotes to avoid remote command injection.
- HookManager.run "mime-decode", :content_type => content_type,
+ HookManager.run "mime-decode", :content_type => @content_type,
:filename => lambda { write_to_disk },
:charset => encoded_content.charset,
:sibling_types => sibling_types
@@ -189,7 +194,7 @@ def view!
## note that the path returned from write_to_disk is
## Shellwords.escaped and is intended to be used without single
## or double quotes. the use of either opens sup up for remote
- ## code injection in the file name.
+ ## code injection through the file name.
def write_to_disk
begin
file = Tempfile.new(["sup", Shellwords.escape(@filename.gsub("/", "_")) || "sup-attachment"])

0 comments on commit ca0302e

Please sign in to comment.