Control: tags -1 - confirmed [Adam D. Barratt] > We'd generally prefer a bit more testing than "should solve the > problem", although I agree that the patch looks sane enough as someone > who knows practically nothing about Ruby... > > Please go ahead.
Thank you. I agree that a bit more testing was needed, but had to struggle a bit to find test code to verify the fix. In the process I discovered that this fix was only fixing half the problem, and added a patch for CVE-2015-7551 and the fiddle code as well. The new and better tested code is attached. The fiddle patch from upstream even had a testsuite fragment to verify its correctness. :) Still OK to upload? Asking again as the changes became twice as large. :) -- Happy hacking Petter Reinholdtsen
>From fdd5802560badf7c4ed0fdbb566dea598ef342a9 Mon Sep 17 00:00:00 2001 From: Petter Reinholdtsen <[email protected]> Date: Tue, 7 Jun 2016 07:31:34 +0200 Subject: [PATCH] Fix CVE-2009-5147 and CVE-2015-7551. Closes: #796344 --- debian/changelog | 12 ++++ debian/patches/CVE-2009-5147.patch | 33 +++++++++++ debian/patches/CVE-2015-7551.patch | 110 +++++++++++++++++++++++++++++++++++++ debian/patches/series | 2 + 4 files changed, 157 insertions(+) create mode 100644 debian/patches/CVE-2009-5147.patch create mode 100644 debian/patches/CVE-2015-7551.patch diff --git a/debian/changelog b/debian/changelog index 13a9637..465f534 100644 --- a/debian/changelog +++ b/debian/changelog @@ -1,3 +1,15 @@ +ruby2.1 (2.1.5-2+deb8u3) jessie; urgency=low + + * Non-maintainer upload to fix security problem. + * Fix CVE-2009-5147: DL::dlopen should not open a library with + tainted library name in safe mode (Closes: #796344). Based on + patch used in DLA-299-1, which was pulled from upstream. + * Fix CVE-2015-7551: Fiddle handles should not call functions with + tainted function names (Closes: #796344). Patch pulled from + upstream. + + -- Petter Reinholdtsen <[email protected]> Tue, 07 Jun 2016 11:00:04 +0200 + ruby2.1 (2.1.5-2+deb8u2) jessie; urgency=high * Apply upstream patches to fix Request hijacking vulnerability in Rubygems diff --git a/debian/patches/CVE-2009-5147.patch b/debian/patches/CVE-2009-5147.patch new file mode 100644 index 0000000..8bdc1d1 --- /dev/null +++ b/debian/patches/CVE-2009-5147.patch @@ -0,0 +1,33 @@ +Description: CVE-2009-5147: DL::dlopen could open a library with tainted library name +Origin: upstream, https://github.com/ruby/ruby/commit/4600cf725a86ce31266153647ae5aa1197b1215b +Debian-Bug: https://bugs.debian.org/796344 +Reviewed-by: Santiago R.R. <[email protected]> +Reviewed-by: Petter Reinholdtsen <[email protected]> + +Index: ruby2.1-2.1.5/ext/dl/handle.c +=================================================================== +--- ruby2.1-2.1.5.orig/ext/dl/handle.c 2016-06-07 07:02:28.284056469 +0200 ++++ ruby2.1-2.1.5/ext/dl/handle.c 2016-06-07 07:02:28.284056469 +0200 +@@ -5,6 +5,8 @@ + #include <ruby.h> + #include "dl.h" + ++#define SafeStringValuePtr(v) (rb_string_value(&v), rb_check_safe_obj(v), RSTRING_PTR(v)) ++ + VALUE rb_cDLHandle; + + #ifdef _WIN32 +@@ -132,11 +134,11 @@ + cflag = RTLD_LAZY | RTLD_GLOBAL; + break; + case 1: +- clib = NIL_P(lib) ? NULL : StringValuePtr(lib); ++ clib = NIL_P(lib) ? NULL : SafeStringValuePtr(lib); + cflag = RTLD_LAZY | RTLD_GLOBAL; + break; + case 2: +- clib = NIL_P(lib) ? NULL : StringValuePtr(lib); ++ clib = NIL_P(lib) ? NULL : SafeStringValuePtr(lib); + cflag = NUM2INT(flag); + break; + default: diff --git a/debian/patches/CVE-2015-7551.patch b/debian/patches/CVE-2015-7551.patch new file mode 100644 index 0000000..a0a1fd8 --- /dev/null +++ b/debian/patches/CVE-2015-7551.patch @@ -0,0 +1,110 @@ +Description: CVE-2015-7551: Add checks to Fiddle for tainted string arguments + Include test case to verify the fix. +Origin: upstream, https://github.com/ruby/ruby/commit/339e11a7f178312d937b7c95dd3115ce7236597a +Debian-Bug: https://bugs.debian.org/796344 +Reviewed-by: Petter Reinholdtsen <[email protected]> + +diff --git a/ext/fiddle/handle.c b/ext/fiddle/handle.c +index 36970a2..fa207ef 100644 +--- a/ext/fiddle/handle.c ++++ b/ext/fiddle/handle.c +@@ -1,6 +1,8 @@ + #include <ruby.h> + #include <fiddle.h> + ++#define SafeStringValueCStr(v) (rb_check_safe_obj(rb_string_value(&v)), StringValueCStr(v)) ++ + VALUE rb_cHandle; + + struct dl_handle { +@@ -143,11 +145,11 @@ rb_fiddle_handle_initialize(int argc, VALUE argv[], VALUE self) + cflag = RTLD_LAZY | RTLD_GLOBAL; + break; + case 1: +- clib = NIL_P(lib) ? NULL : StringValuePtr(lib); ++ clib = NIL_P(lib) ? NULL : SafeStringValueCStr(lib); + cflag = RTLD_LAZY | RTLD_GLOBAL; + break; + case 2: +- clib = NIL_P(lib) ? NULL : StringValuePtr(lib); ++ clib = NIL_P(lib) ? NULL : SafeStringValueCStr(lib); + cflag = NUM2INT(flag); + break; + default: +@@ -263,7 +265,7 @@ rb_fiddle_handle_to_i(VALUE self) + return PTR2NUM(fiddle_handle); + } + +-static VALUE fiddle_handle_sym(void *handle, const char *symbol); ++static VALUE fiddle_handle_sym(void *handle, VALUE symbol); + + /* + * Document-method: sym +@@ -282,7 +284,7 @@ rb_fiddle_handle_sym(VALUE self, VALUE sym) + rb_raise(rb_eFiddleError, "closed handle"); + } + +- return fiddle_handle_sym(fiddle_handle->ptr, StringValueCStr(sym)); ++ return fiddle_handle_sym(fiddle_handle->ptr, sym); + } + + #ifndef RTLD_NEXT +@@ -305,11 +307,11 @@ rb_fiddle_handle_sym(VALUE self, VALUE sym) + static VALUE + rb_fiddle_handle_s_sym(VALUE self, VALUE sym) + { +- return fiddle_handle_sym(RTLD_NEXT, StringValueCStr(sym)); ++ return fiddle_handle_sym(RTLD_NEXT, sym); + } + + static VALUE +-fiddle_handle_sym(void *handle, const char *name) ++fiddle_handle_sym(void *handle, VALUE symbol) + { + #if defined(HAVE_DLERROR) + const char *err; +@@ -318,6 +320,7 @@ fiddle_handle_sym(void *handle, const char *name) + # define CHECK_DLERROR + #endif + void (*func)(); ++ const char *name = SafeStringValueCStr(symbol); + + rb_secure(2); + #ifdef HAVE_DLERROR +@@ -367,7 +370,7 @@ fiddle_handle_sym(void *handle, const char *name) + } + #endif + if( !func ){ +- rb_raise(rb_eFiddleError, "unknown symbol \"%s\"", name); ++ rb_raise(rb_eFiddleError, "unknown symbol \"%"PRIsVALUE"\"", symbol); + } + + return PTR2NUM(func); +diff --git a/test/fiddle/test_handle.rb b/test/fiddle/test_handle.rb +index 2007a19..8d7589e 100644 +--- a/test/fiddle/test_handle.rb ++++ b/test/fiddle/test_handle.rb +@@ -10,6 +10,23 @@ class TestHandle < TestCase + + include Test::Unit::Assertions + ++ def test_safe_handle_open ++ t = Thread.new do ++ $SAFE = 1 ++ Fiddle::Handle.new(LIBC_SO.taint) ++ end ++ assert_raise(SecurityError) { t.value } ++ end ++ ++ def test_safe_function_lookup ++ t = Thread.new do ++ h = Fiddle::Handle.new(LIBC_SO) ++ $SAFE = 1 ++ h["qsort".taint] ++ end ++ assert_raise(SecurityError) { t.value } ++ end ++ + def test_to_i + handle = Fiddle::Handle.new(LIBC_SO) + assert_kind_of Integer, handle.to_i diff --git a/debian/patches/series b/debian/patches/series index 7bb8252..ec4b983 100644 --- a/debian/patches/series +++ b/debian/patches/series @@ -1 +1,3 @@ debian-changes +CVE-2009-5147.patch +CVE-2015-7551.patch -- 2.1.4
