[ruby-core:65839] [ruby-trunk - Feature #10351] [PATCH] prevent CVE-2014-6277

Issue #10351 has been updated by Shyouhei Urabe.


 I see this patch is (basically) good to have.
 
 One thing sprung in my mind is that perhaps we should also freeze each environment variables, not only the ENV object.  That is not the same way Hash#freeze works, but ENV keys are already frozen as-is so also freezing values might be an opiton.  That should prevent modifying already-existent environment variables, such as RUBYOPT.
 
 On 10/21/2014 05:24 AM, Eric Wong wrote:
 > How about supporting ENV.freeze instead?
 > Currently ENV.freeze is a no-op, this patch changes that:
 > 
 > http://80x24.org/spew/m/400e216159e74b65608f2f0b296817cc9823e3bb.txt
 > 
 >     ENV.freeze should behave like Hash#freeze and not allow future
 >     modifications to the ENV from pure Ruby libraries.
 > 
 >     Users may call ENV.freeze to prevent 3rd-party (pure) Ruby libraries
 >     from modifying the process environment any further.
 > 
 >     This cannot not defend against users who modify the environment using
 >     3rd-party C extensions, Fiddle, or FFI RubyGem
 > 
 >

----------------------------------------
Feature #10351: [PATCH] prevent CVE-2014-6277
https://bugs.ruby-lang.org/issues/10351#change-49574

* Author: Shyouhei Urabe
* Status: Feedback
* Priority: Normal
* Assignee: 
* Category: core
* Target version: current: 2.2.0
----------------------------------------
~~~
From 4636ca0308f1933c9b191f36e808a8d3bcf5e88e Mon Sep 17 00:00:00 2001
From: "Urabe, Shyouhei" <[e-mail]>
Date: Wed, 8 Oct 2014 15:44:27 +0900
Subject: [PATCH] prevent CVE-2014-6277

ShellShock was about bash.  I think ruby is torellant for that kind of
attack.  However the concept was that environment variables can
potentially contain malicious data.  To pass them to victim
subprocecsses can sometimes cause catastrophic situation, like
arbitrary code execution.  Even though ruby itself is not affected, it
can be an accomplice in attacks by blindly passing through any
uncontrolled info through.  Let's just change this, more secure by
default.

This patch does not add a new feature, nor delete anything.  It just
changes the default behaviour when ruby spawns subprocesses.

    Process.spawn('/usr/bin/printenv') # -> prints nothing

You can explicitly pass what you need:

    Process.spawn({'FOO'=>'BAR'}, '/usr/bin/printenv')

Or if you are 128% sure what you are doing, can pass everything.

    ENV['FOO'] = 'BAR'
    Process.spawn('/usr/bin/printenv', unsetenv_others: false)

I don't intend to make things impossible; just give it a better
default.  It doesn't ultimately solve everything but it should prevent
casual faults.

---
 process.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/process.c b/process.c
index f9bc01c..395333d 100644
--- a/process.c
+++ b/process.c
@@ -2295,7 +2295,7 @@ rb_execarg_fixup(VALUE execarg_obj)
         eargp->dup2_tmpbuf = tmpbuf;
     }
 
-    unsetenv_others = eargp->unsetenv_others_given && eargp->unsetenv_others_do;
+    unsetenv_others = !eargp->unsetenv_others_given || eargp->unsetenv_others_do;
     envopts = eargp->env_modification;
     if (unsetenv_others || envopts != Qfalse) {
         VALUE envtbl, envp_str, envp_buf;
@@ -2936,7 +2936,7 @@ rb_execarg_run_options(const struct rb_execarg *eargp, struct rb_execarg *sargp,
 #endif
 
 #if !defined(HAVE_WORKING_FORK)
-    if (eargp->unsetenv_others_given && eargp->unsetenv_others_do) {
+    if (!eargp->unsetenv_others_given || eargp->unsetenv_others_do) {
         save_env(sargp);
         rb_env_clear();
     }
@@ -3998,8 +3998,8 @@ rb_f_system(int argc, VALUE *argv)
  *      [cmdname, argv0], arg1, ... : command name, argv[0] and zero or more arguments (no shell)
  *    options: hash
  *      clearing environment variables:
- *        :unsetenv_others => true   : clear environment variables except specified by env
- *        :unsetenv_others => false  : don't clear (default)
+ *        :unsetenv_others => true   : clear environment variables except specified by env (default)
+ *        :unsetenv_others => false  : don't clear
  *      process group:
  *        :pgroup => true or 0 : make a new process group
  *        :pgroup => pgid      : join to specified process group
-- 
1.9.1
~~~



-- 
https://bugs.ruby-lang.org/