Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Apply escaping args to other command shells #19745

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions lib/msf/base/sessions/command_shell.rb
Original file line number Diff line number Diff line change
Expand Up @@ -213,6 +213,11 @@ def cmd_background_help
print_line
end

def escape_arg(arg)
# By default we don't know what the escaping is. It's not ideal, but subclasses should do their own appropriate escaping
arg
end

def cmd_background(*args)
if !args.empty?
# We assume that background does not need arguments
Expand Down
39 changes: 2 additions & 37 deletions lib/msf/base/sessions/command_shell_unix.rb
Original file line number Diff line number Diff line change
Expand Up @@ -6,43 +6,8 @@ def initialize(*args)
super
end

def shell_command_token(cmd,timeout = 10)
shell_command_token_unix(cmd,timeout)
end

# Convert the executable and argument array to a command that can be run in this command shell
# @param cmd_and_args [Array<String>] The process path and the arguments to the process
def to_cmd(cmd_and_args)
self.class.to_cmd(cmd_and_args)
end

# Escape an individual argument per Unix shell rules
# @param arg [String] Shell argument
def escape_arg(arg)
self.class.escape_arg(arg)
end

# Convert the executable and argument array to a command that can be run in this command shell
# @param cmd_and_args [Array<String>] The process path and the arguments to the process
def self.to_cmd(cmd_and_args)
escaped = cmd_and_args.map do |arg|
escape_arg(arg)
end

escaped.join(' ')
end

# Escape an individual argument per Unix shell rules
# @param arg [String] Shell argument
def self.escape_arg(arg)
quote_requiring = ['\\', '`', '(', ')', '<', '>', '&', '|', ' ', '@', '"', '$', ';']
result = CommandShell._glue_cmdline_escape(arg, quote_requiring, "'", "\\'", "'")
if result == ''
result = "''"
end

result
end
include Msf::Sessions::UnixEscaping
extend Msf::Sessions::UnixEscaping
end

end
111 changes: 2 additions & 109 deletions lib/msf/base/sessions/command_shell_windows.rb
Original file line number Diff line number Diff line change
Expand Up @@ -6,114 +6,7 @@ def initialize(*args)
super
end

def self.space_chars
[' ', '\t', '\v']
end

def shell_command_token(cmd,timeout = 10)
shell_command_token_win32(cmd,timeout)
end

# Convert the executable and argument array to a command that can be run in this command shell
# @param cmd_and_args [Array<String>] The process path and the arguments to the process
def to_cmd(cmd_and_args)
self.class.to_cmd(cmd_and_args)
end

# Escape a process for the command line
# @param executable [String] The process to launch
def self.escape_cmd(executable)
needs_quoting = space_chars.any? do |char|
executable.include?(char)
end

if needs_quoting
executable = "\"#{executable}\""
end

executable
end

# Convert the executable and argument array to a commandline that can be passed to CreateProcessAsUserW.
# @param args [Array<String>] The arguments to the process
# @remark The difference between this and `to_cmd` is that the output of `to_cmd` is expected to be passed
# to cmd.exe, whereas this is expected to be passed directly to the Win32 API, anticipating that it
# will in turn be interpreted by CommandLineToArgvW.
def self.argv_to_commandline(args)
escaped_args = args.map do |arg|
escape_arg(arg)
end

escaped_args.join(' ')
end

# Escape an individual argument per Windows shell rules
# @param arg [String] Shell argument
def self.escape_arg(arg)
needs_quoting = space_chars.any? do |char|
arg.include?(char)
end

# Fix the weird behaviour when backslashes are treated differently when immediately prior to a double-quote
# We need to send double the number of backslashes to make it work as expected
# See: https://learn.microsoft.com/en-us/windows/win32/api/shellapi/nf-shellapi-commandlinetoargvw#remarks
arg = arg.gsub(/(\\*)"/, '\\1\\1"')

# Quotes need to be escaped
arg = arg.gsub('"', '\\"')

if needs_quoting
# At the end of the argument, we're about to add another quote - so any backslashes need to be doubled here too
arg = arg.gsub(/(\\*)$/, '\\1\\1')
arg = "\"#{arg}\""
end

# Empty string needs to be coerced to have a value
arg = '""' if arg == ''

arg
end

# Convert the executable and argument array to a command that can be run in this command shell
# @param cmd_and_args [Array<String>] The process path and the arguments to the process
def self.to_cmd(cmd_and_args)
# The space, caret and quote chars need to be inside double-quoted strings.
# The percent character needs to be escaped using a caret char, while being outside a double-quoted string.
#
# Situations where these two situations combine are going to be the trickiest cases: something that has quote-requiring
# characters (e.g. spaces), but which also needs to avoid expanding an environment variable. In this case,
# the string needs to end up being partially quoted; with parts of the string in quotes, but others (i.e. bits with percents) not.
# For example:
# 'env var is %temp%, yes, %TEMP%' needs to end up as '"env var is "^%temp^%", yes, "^%TEMP^%'
#
# There is flexibility in how you might implement this, but I think this one looks the most "human" to me,
# which would make it less signaturable.
#
# To do this, we'll consider each argument character-by-character. Each time we encounter a percent sign, we break out of any quotes
# (if we've been inside them in the current "token"), and then start a new "token".

quote_requiring = ['"', '^', ' ', "\t", "\v", '&', '<', '>', '|']

escaped_cmd_and_args = cmd_and_args.map do |arg|
# Escape quote chars by doubling them up, except those preceeded by a backslash (which are already effectively escaped, and handled below)
arg = arg.gsub(/([^\\])"/, '\\1""')
arg = arg.gsub(/^"/, '""')

result = CommandShell._glue_cmdline_escape(arg, quote_requiring, '%', '^%', '"')

# Fix the weird behaviour when backslashes are treated differently when immediately prior to a double-quote
# We need to send double the number of backslashes to make it work as expected
# See: https://learn.microsoft.com/en-us/windows/win32/api/shellapi/nf-shellapi-commandlinetoargvw#remarks
result.gsub!(/(\\*)"/, '\\1\\1"')

# Empty string needs to be coerced to have a value
result = '""' if result == ''

result
end

escaped_cmd_and_args.join(' ')
end
include Msf::Sessions::WindowsEscaping
extend Msf::Sessions::WindowsEscaping
end

end
5 changes: 5 additions & 0 deletions lib/msf/base/sessions/ssh_command_shell_bind.rb
Original file line number Diff line number Diff line change
Expand Up @@ -238,6 +238,11 @@ def initialize(ssh_connection, opts = {})
def bootstrap(datastore = {}, handler = nil)
# this won't work after the rstream is initialized, so do it first
@platform = Metasploit::Framework::Ssh::Platform.get_platform(ssh_connection)
if @platform == 'windows'
extend(Msf::Sessions::WindowsEscaping)
else
extend(Msf::Sessions::UnixEscaping)
end
Comment on lines +241 to +245
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are you sure there are no other possibilities besides Windows and Unix wrt. shell escaping?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From https://github.com/rapid7/metasploit-framework/blob/master/lib/metasploit/framework/ssh/platform.rb#L97-L132, I'm really not all that familiar with what exactly to expect shell-escaping-wise when you get (for example) an "arista" shell. If it's some custom shell completely unrelated to anything POSIX, then any of the behaviour that uses escape_arg (such as uploading files or launching processes) would be useless anyway and would need its entirely new implementation.

In those cases, I'm not sure what the best default would be. The alternative I guess would be better in those unknown cases to just not extend the object, and do no escaping.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From my very brief analysis, the current state should easily cover all platforms except Cisco ios, Juniper, Mikrotik, Arista and vcenter - I'm not 100% sure how those shells work when it comes to character escaping. So the options are: add else for all above mentioned and just implement escaping for them later on with more thorough analysis or ignore it, hope for best and add at least else block for Unknown platform. We will probably have to create/extend tests for SSH sessions (though I'm not very familiar how they are implemented now, but I don't expect such cases are covered there).


# if the platform is known, it was recovered by communicating with the device, so skip verification, also not all
# shells accessed through SSH may respond to the echo command issued for verification as expected
Expand Down
27 changes: 27 additions & 0 deletions lib/msf/base/sessions/unix_escaping.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
module Msf::Sessions
module UnixEscaping
def shell_command_token(cmd,timeout = 10)
shell_command_token_unix(cmd,timeout)
end

# Convert the executable and argument array to a command that can be run in this command shell
# @param cmd_and_args [Array<String>] The process path and the arguments to the process
def to_cmd(cmd_and_args)
escaped = cmd_and_args.map { |arg| escape_arg(arg) }

escaped.join(' ')
end

# Escape an individual argument per Unix shell rules
# @param arg [String] Shell argument
def escape_arg(arg)
quote_requiring = ['\\', '`', '(', ')', '<', '>', '&', '|', ' ', '@', '"', '$', ';']
result = CommandShell._glue_cmdline_escape(arg, quote_requiring, "'", "\\'", "'")
if result == ''
result = "''"
end

result
end
end
end
106 changes: 106 additions & 0 deletions lib/msf/base/sessions/windows_escaping.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,106 @@
module Msf::Sessions
module WindowsEscaping
def space_chars
[' ', '\t', '\v']
end

def shell_command_token(cmd,timeout = 10)
shell_command_token_win32(cmd,timeout)
end

# Escape a process for the command line
# @param executable [String] The process to launch
def escape_cmd(executable)
needs_quoting = space_chars.any? do |char|
executable.include?(char)
end

if needs_quoting
executable = "\"#{executable}\""
end

executable
end

# Convert the executable and argument array to a commandline that can be passed to CreateProcessAsUserW.
# @param args [Array<String>] The arguments to the process
# @remark The difference between this and `to_cmd` is that the output of `to_cmd` is expected to be passed
# to cmd.exe, whereas this is expected to be passed directly to the Win32 API, anticipating that it
# will in turn be interpreted by CommandLineToArgvW.
def argv_to_commandline(args)
escaped_args = args.map do |arg|
escape_arg(arg)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

More condensed form

Suggested change
escape_arg(arg)
escaped_args = args.map { |arg| escape_arg(arg) }

end

escaped_args.join(' ')
end

# Escape an individual argument per Windows shell rules
# @param arg [String] Shell argument
def escape_arg(arg)
needs_quoting = space_chars.any? do |char|
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

More condensed form

Suggested change
needs_quoting = space_chars.any? do |char|
needs_quoting = space_chars.any? { |char| arg.include?(char) }

arg.include?(char)
end

# Fix the weird behaviour when backslashes are treated differently when immediately prior to a double-quote
# We need to send double the number of backslashes to make it work as expected
# See: https://learn.microsoft.com/en-us/windows/win32/api/shellapi/nf-shellapi-commandlinetoargvw#remarks
arg = arg.gsub(/(\\*)"/, '\\1\\1"')

# Quotes need to be escaped
arg = arg.gsub('"', '\\"')

if needs_quoting
# At the end of the argument, we're about to add another quote - so any backslashes need to be doubled here too
arg = arg.gsub(/(\\*)$/, '\\1\\1')
arg = "\"#{arg}\""
end

# Empty string needs to be coerced to have a value
arg = '""' if arg == ''

arg
end

# Convert the executable and argument array to a command that can be run in this command shell
# @param cmd_and_args [Array<String>] The process path and the arguments to the process
def to_cmd(cmd_and_args)
# The space, caret and quote chars need to be inside double-quoted strings.
# The percent character needs to be escaped using a caret char, while being outside a double-quoted string.
#
# Situations where these two situations combine are going to be the trickiest cases: something that has quote-requiring
# characters (e.g. spaces), but which also needs to avoid expanding an environment variable. In this case,
# the string needs to end up being partially quoted; with parts of the string in quotes, but others (i.e. bits with percents) not.
# For example:
# 'env var is %temp%, yes, %TEMP%' needs to end up as '"env var is "^%temp^%", yes, "^%TEMP^%'
#
# There is flexibility in how you might implement this, but I think this one looks the most "human" to me,
# which would make it less signaturable.
#
# To do this, we'll consider each argument character-by-character. Each time we encounter a percent sign, we break out of any quotes
# (if we've been inside them in the current "token"), and then start a new "token".

quote_requiring = ['"', '^', ' ', "\t", "\v", '&', '<', '>', '|']

escaped_cmd_and_args = cmd_and_args.map do |arg|
# Escape quote chars by doubling them up, except those preceeded by a backslash (which are already effectively escaped, and handled below)
arg = arg.gsub(/([^\\])"/, '\\1""')
arg = arg.gsub(/^"/, '""')

result = CommandShell._glue_cmdline_escape(arg, quote_requiring, '%', '^%', '"')

# Fix the weird behaviour when backslashes are treated differently when immediately prior to a double-quote
# We need to send double the number of backslashes to make it work as expected
# See: https://learn.microsoft.com/en-us/windows/win32/api/shellapi/nf-shellapi-commandlinetoargvw#remarks
result.gsub!(/(\\*)"/, '\\1\\1"')

# Empty string needs to be coerced to have a value
result = '""' if result == ''

result
end

escaped_cmd_and_args.join(' ')
end
end
end
Loading