ruby-changes:71207
From: Burdette <ko1@a...>
Date: Fri, 18 Feb 2022 21:46:17 +0900 (JST)
Subject: [ruby-changes:71207] e9a2b30744 (master): Enhanced RDoc concerning command injection (#5537)
https://git.ruby-lang.org/ruby.git/commit/?id=e9a2b30744 From e9a2b30744a62268c66d6c17730ed96486d9783c Mon Sep 17 00:00:00 2001 From: Burdette Lamar <BurdetteLamar@Y...> Date: Fri, 18 Feb 2022 06:46:04 -0600 Subject: Enhanced RDoc concerning command injection (#5537) Clarifies security vulnerabilities for commands. Treats: Kernel.system Kernel.` (backtick) IO.popen IO.read IO.write IO.binread IO.binwrite IO.readlines IO.foreach --- doc/command_injection.rdoc | 29 +++++++++++++++++++ io.c | 71 ++++++++++++++++++++++++++++++---------------- object.c | 2 +- process.c | 3 ++ 4 files changed, 79 insertions(+), 26 deletions(-) create mode 100644 doc/command_injection.rdoc diff --git a/doc/command_injection.rdoc b/doc/command_injection.rdoc new file mode 100644 index 0000000000..8f1303bcf7 --- /dev/null +++ b/doc/command_injection.rdoc @@ -0,0 +1,29 @@ https://github.com/ruby/ruby/blob/trunk/doc/command_injection.rdoc#L1 +== Command Injection + +Some Ruby core methods accept string data +that includes text to be executed as a system command. + +They should not be called with unknown or unsanitized commands. + +These methods include: + +- Kernel.system +- {`command` (backtick method)}[rdoc-ref:Kernel#`] + (also called by the expression <tt>%x[command]</tt>). +- IO.popen(command). +- IO.read(command). +- IO.write(command). +- IO.binread(command). +- IO.binwrite(command). +- IO.readlines(command). +- IO.foreach(command). + +Note that some of these methods do not execute commands when called +from subclass \File: + +- File.read(path). +- File.write(path). +- File.binread(path). +- File.binwrite(path). +- File.readlines(path). +- File.foreach(path). diff --git a/io.c b/io.c index 88897960ab..24488d2db7 100644 --- a/io.c +++ b/io.c @@ -7374,6 +7374,9 @@ static VALUE popen_finish(VALUE port, VALUE klass); https://github.com/ruby/ruby/blob/trunk/io.c#L7374 * Executes the given command +cmd+ as a subprocess * whose $stdin and $stdout are connected to a new stream +io+. * + * This method has potential security vulnerabilities if called with untrusted input; + * see {Command Injection}[command_injection.rdoc]. + * * If no block is given, returns the new stream, * which depending on given +mode+ may be open for reading, writing, or both. * The stream should be explicitly closed (eventually) to avoid resource leaks. @@ -9813,10 +9816,15 @@ argf_readlines(int argc, VALUE *argv, VALUE argf) https://github.com/ruby/ruby/blob/trunk/io.c#L9816 /* * call-seq: - * `cmd` -> string + * `command` -> string + * + * Returns the <tt>$stdout</tt> output from running +command+ in a subshell; + * sets global variable <tt>$?</tt> to the process status. * - * Returns the <tt>$stdout</tt> output fromm running +cmd+ in a subshell; - * sets global variable <tt>$?</tt> to the process status: + * This method has potential security vulnerabilities if called with untrusted input; + * see {Command Injection}[command_injection.rdoc]. + * + * Examples: * * $ `date` # => "Wed Apr 9 08:56:30 CDT 2003\n" * $ `echo oops && exit 99` # => "oops\n" @@ -11196,36 +11204,29 @@ io_s_foreach(VALUE v) https://github.com/ruby/ruby/blob/trunk/io.c#L11204 /* * call-seq: - * IO.foreach(command, sep = $/, **opts) {|line| block } -> nil - * IO.foreach(command, limit, **opts) {|line| block } -> nil - * IO.foreach(command, sep, limit, **opts) {|line| block } -> nil * IO.foreach(path, sep = $/, **opts) {|line| block } -> nil * IO.foreach(path, limit, **opts) {|line| block } -> nil * IO.foreach(path, sep, limit, **opts) {|line| block } -> nil + * IO.foreach(command, sep = $/, **opts) {|line| block } -> nil + * IO.foreach(command, limit, **opts) {|line| block } -> nil + * IO.foreach(command, sep, limit, **opts) {|line| block } -> nil * IO.foreach(...) -> an_enumerator * * Calls the block with each successive line read from the stream. * - * The first argument must be a string; - * its meaning depends on whether it starts with the pipe character (<tt>'|'</tt>): + * When called from class \IO (but not subclasses of \IO), + * this method has potential security vulnerabilities if called with untrusted input; + * see {Command Injection}[command_injection.rdoc]. * - * - If so (and if +self+ is \IO), - * the rest of the string is a command to be executed as a subprocess. - * - Otherwise, the string is the path to a file. + * The first argument must be a string that is one of the following: * - * With only argument +command+ given, executes the command in a shell, - * parses its $stdout into lines, as determined by the default line separator, - * and calls the block with each successive line: - * - * IO.foreach('| cat t.txt') {|line| p line } - * - * Output: - * - * "First line\n" - * "Second line\n" - * "\n" - * "Third line\n" - * "Fourth line\n" + * - Path: if +self+ is a subclass of \IO (\File, for example), + * or if the string _does_ _not_ start with the pipe character (<tt>'|'</tt>), + * the string is the path to a file. + * - Command: if +self+ is the class \IO, + * and if the string starts with the pipe character, + * the rest of the string is a command to be executed as a subprocess. + * See the {Note on Security}[@Note+on+Security]. * * With only argument +path+ given, parses lines from the file at the given +path+, * as determined by the default line separator, @@ -11327,6 +11328,10 @@ io_s_readlines(VALUE v) https://github.com/ruby/ruby/blob/trunk/io.c#L11328 * * Returns an array of all lines read from the stream. * + * When called from class \IO (but not subclasses of \IO), + * this method has potential security vulnerabilities if called with untrusted input; + * see {Command Injection}[command_injection.rdoc]. + * * The first argument must be a string; * its meaning depends on whether it starts with the pipe character (<tt>'|'</tt>): * @@ -11427,6 +11432,10 @@ seek_before_access(VALUE argp) https://github.com/ruby/ruby/blob/trunk/io.c#L11432 * Opens the stream, reads and returns some or all of its content, * and closes the stream; returns +nil+ if no bytes were read. * + * When called from class \IO (but not subclasses of \IO), + * this method has potential security vulnerabilities if called with untrusted input; + * see {Command Injection}[command_injection.rdoc]. + * * The first argument must be a string; * its meaning depends on whether it starts with the pipe character (<tt>'|'</tt>): * @@ -11498,6 +11507,10 @@ rb_io_s_read(int argc, VALUE *argv, VALUE io) https://github.com/ruby/ruby/blob/trunk/io.c#L11507 * Behaves like IO.read, except that the stream is opened in binary mode * with ASCII-8BIT encoding. * + * When called from class \IO (but not subclasses of \IO), + * this method has potential security vulnerabilities if called with untrusted input; + * see {Command Injection}[command_injection.rdoc]. + * */ static VALUE @@ -11599,10 +11612,14 @@ io_s_write(int argc, VALUE *argv, VALUE klass, int binary) https://github.com/ruby/ruby/blob/trunk/io.c#L11612 * Opens the stream, writes the given +data+ to it, * and closes the stream; returns the number of bytes written. * + * When called from class \IO (but not subclasses of \IO), + * this method has potential security vulnerabilities if called with untrusted input; + * see {Command Injection}[command_injection.rdoc]. + * * The first argument must be a string; * its meaning depends on whether it starts with the pipe character (<tt>'|'</tt>): * - * - If so (and if +self+ is an instance of \IO), + * - If so (and if +self+ is \IO), * the rest of the string is a command to be executed as a subprocess. * - Otherwise, the string is the path to a file. * @@ -11660,6 +11677,10 @@ rb_io_s_write(int argc, VALUE *argv, VALUE io) https://github.com/ruby/ruby/blob/trunk/io.c#L11677 * Behaves like IO.write, except that the stream is opened in binary mode * with ASCII-8BIT encoding. * + * When called from class \IO (but not subclasses of \IO), + * this method has potential security vulnerabilities if called with untrusted input; + * see {Command Injection}[command_injection.rdoc]. + * */ static VALUE diff --git a/object.c b/object.c index 33715a5714..f056767f13 100644 --- a/object.c +++ b/object.c @@ -4389,7 +4389,7 @@ InitVM_Object(void) https://github.com/ruby/ruby/blob/trunk/object.c#L4389 * * === Subprocesses * - * - #`cmd`:: Returns the standard output of running +cmd+ in a subshell. + * - #`command`:: Returns the standard output of running +command+ in a subshell. * - #exec:: Replaces current process with a new process. * - #fork:: Forks the current process into two processes. * - #spawn:: Executes the given command and returns its pid without waiting diff --git a/process.c b/process.c index 8eee6f772a..4011aa569f 100644 --- a/process.c +++ b/process.c @@ -4755,6 +4755,9 @@ rb_spawn(int argc, const VALUE *argv) https://github.com/ruby/ruby/blob/trunk/process.c#L4755 * Executes _command..._ in a subshell. * _command..._ is one of following forms. * + * This method has potential security vulnerabilities if called with untrusted input; + * see {Command Injection}[command_injection.rdoc]. + * * [<code>commandline</code>] * command line string which is passed to the standard shell * [<code>cmdname, arg1, ...</code>] -- cgit v1.2.1 -- ML: ruby-changes@q... Info: http://www.atdot.net/~ko1/quickml/