Ruby 1.9.0/1.8.7/1.8.6/1.8.5 new releases (Security Fix)

Igal Koshevoy wrote:

Phil Ross wrote:

It looks like a fix for the segmentation faults was committed on 21 June
(revision 17530 in the ruby_1_8 branch):

Phil, thanks for posting the link and patch.

Unfortunately, this patched version fails with segmentation faults when applied to 1.8.6-p230 and run against RSpec 1.1.4's test suite:

  lib/spec/example/example_group_methods.rb:384: [BUG] Segmentation fault

The 1.8.6-p111 version doesn't segfault.

Igal,

Thanks for testing this.

It would appear that there are (at least) two separate areas where
segmentation faults are being raised. Revision 17530 fixes segmentation
faults with string concatenation that was causing my Rails applications
to crash:

http://www.ruby-forum.com/topic/157250

I guess that the root cause must be revision 17222 in the ruby_1_8_6
branch that the smartleaf patch reverts.

Regards,

Phil

···

--
Phil Ross
http://tzinfo.rubyforge.org/ -- DST-aware timezone library for Ruby

  http://dev.smartleaf.com/misc/p230_fixit_patch.txt

This reverts changeset 17222 from the ruby_1_8_6 branch of the
main svn repository, which doesn't *look* security-related, at
least at first blush (though it may be a failed backport from
another line of development).

I ran this against the Rails 2.0 and RSpec 1.1.4 test
suites, no seg faults, no glibc errs, and the same set of tests
succeeded/passed between this patched version and the stock p111. It ran
fine against automateit 0.80607 and the various Rails apps I tried. This
is good.

I was running Ruby-1.8.6-p230 with this patch for about a week on our
multi-rails-app server, but then came across a problem in an old Rails
app (originally Rails 1.1.6, which we upgraded to 1.2.6 in a failed
attempt at fixing this).

The problem was in file uploads from an Internet Explorer client. The
file object's original_filename method was returning just 'Documents,
and the full_original_filename method was returning just '"C:\Documents'
(including the initial double-quotes, but not including the single
quotes), when someone uploaded a file from under their 'C:\Documents and
Settings\' path.

I switched to Ruby-1.8.6-p111 with the security patches posted above,
and the problem went away.

···

--
Posted via http://www.ruby-forum.com/\.

Ollivier Robert wrote:

Try this instead:
http://www.freebsd.org/cgi/cvsweb.cgi/ports/lang/ruby18/files/

Thanks for the assistance. That FreeBSD web site's UI sucks. Their "Get
diffs" button is broken and always returns nothing. To get a diff on a
file, one must click the "text" next to the revision number.

FreeBSD's backported patch seems insufficient and vulnerable. I come to
this conclusion because they only modified two files (sprintf.c and
string.c) -- but the Ruby changelog for this fix mentions other files
(e.g., array.c), and Zed Shaw identifies about a dozen files potentially
involved in the fix at
http://www.zedshaw.com/rants/the_big_ruby_vulnerabilities.html

So we still need to come up with either a backport for one of the
working versions of Ruby, or a fix to one of the currently released but
broken versions.

I've sent email to Stas, the FreeBSD maintainer of Ruby to warn them of
the potential security hole in their release and in hopes that they may
join this discussion.

-igal

···

--
Posted via http://www.ruby-forum.com/\.

Stanislav Sedov wrote:

...

Thanks for the updates.

I also figured out what I was missing with the patch listing at the
FreeBSD site. When I was hitting page down to get to the files, I ended
up only looking at the last four files and jumped to the incorrect
conclusion that the listing was sorted by chronological order, and thus
thought that you only patched two files based on the dates listed.
However, the listing is actually alpha sorted and I can now see that you
patched other files. Sorry for the silly mistake, it's been a long
night. :slight_smile:

-igal

···

--
Posted via http://www.ruby-forum.com/\.

Thomas Hurst wrote:

rspec runs fine, though I needed to modify a regexp to work with my
Oniguruma patched install (an option of the FreeBSD port).

The Rails test suite mostly works; few failures wrt timezone support,
and a couple of odd ActiveRecord ones with sanitizing LIMIT
(add_limit_offset_should_sanitize_sql_injection_for_limit...), but
these could also be Oniguruma related.

Thanks for the update.

However, does anyone know how the FreeBSD maintainers figured out what
to backport and what not to?

Well, you just follow the SVN history and cherry-pick the relevent
commits?

The intent of my question was to get information so we could evaluate
their selection process, and thus determine whether that process would
effectively included the applicable changes. :slight_smile:

We're currently depending on the assumption that one person cherry
picked all the right commits, and we've already identified at least one
potential error from that process. I'm sure that Stanislav Sedov did a
fine job, but I'd like to see someone else do a second, independent pass
through the history to double-check. I wouldn't trust myself to do
something this important on my own in a single pass, so this is in no
way a criticism.

Would anyone like to volunteer?

-igal

···

--
Posted via http://www.ruby-forum.com/\.

We have another potential winning solution!

Can someone please review this patch and provide advice of the pros/cons
of using this solution versus the Smartleaf patch described in the
previous email? In a nutshell, Stanislav and Hongli's solution is a
backport of fixes to p111, while the Smartleaf solution fixes the
segfaults in p230.

Also, can anyone get through to the official Ruby maintainers? It's
awesome that we can create two unofficial patches on short notice like
this, but I'd really like to have them involved in this.

Hongli Lai wrote:

Now that you mention it, Keita Yamaguchi sent me an eval.c security
patch a while back. Upon closer inspection it seems that this patch is
not included in the FreeBSD patch set, and neither is bignum.c.

The analysis Zed Shaw described in his blog was based on reviewing all
the changes made this month. Although this is more time consuming, it
also seems like the most methodical way of making sure we catch all the
relevant changes.

I've made an updated patch set:
http://blog.phusion.nl/assets/r8ee-security-patch-20080623-2.txt

I ran this against the Rails 2.0 and RSpec 1.1.4 test
suites, no seg faults, no glibc errs, and the same set of tests
succeeded/passed between this patched version and the stock p111.
Excellent.

I also ran this and the smartleaf version against RubySpecs, and got
identical results to those of a stock p111. Excellent.

As far as I can tell, both of these patches provide a working version of
MRI Ruby. Stanislav and Hongli's p111 backport relies on them having
correctly cherry-picked the right fixes. Smartleaf's p230 fix relies on
the rest of that Ruby version to be correct, and I have some concerns
about other lurking bugs if they shipped a version with such an obvious
flaw.

How do we choose between these?

-igal

···

--
Posted via http://www.ruby-forum.com/\.

Phil Ross wrote:

It would appear that there are (at least) two separate areas where
segmentation faults are being raised. Revision 17530 fixes segmentation
faults with string concatenation that was causing my Rails applications
to crash:

http://www.ruby-forum.com/topic/157250

I guess that the root cause must be revision 17222 in the ruby_1_8_6
branch that the smartleaf patch reverts.

Ah, thanks for the update.

So before we discard the 17222 patch too casually ... can anyone explain
what Matz was trying to do there? He clearly went through a fair amount
of effort to try to copy those crefs, but why? And does anything else
depend on that change?

-igal

···

--
Posted via http://www.ruby-forum.com/\.

So I've seen a bunch of different ideas about what folks should
use...but is there a definitive set of sources to use at this point.
Given the short anecdote above and from reading past posts... it seems
like I should give 1.8.6-p111 with the patch above a try.

Any reasons not to?

Mike B.

···

On Jul 3, 11:45 am, Rob Funk <rf...@funknet.net> wrote:

>> http://dev.smartleaf.com/misc/p230_fixit_patch.txt

>> This reverts changeset 17222 from the ruby_1_8_6 branch of the
>> main svn repository, which doesn't *look* security-related, at
>> least at first blush (though it may be a failed backport from
>> another line of development).
> I ran this against the Rails 2.0 and RSpec 1.1.4 test
> suites, no seg faults, no glibc errs, and the same set of tests
> succeeded/passed between this patched version and the stock p111. It ran
> fine against automateit 0.80607 and the various Rails apps I tried. This
> is good.

I was running Ruby-1.8.6-p230 with this patch for about a week on our
multi-rails-app server, but then came across a problem in an old Rails
app (originally Rails 1.1.6, which we upgraded to 1.2.6 in a failed
attempt at fixing this).

The problem was in file uploads from an Internet Explorer client. The
file object's original_filename method was returning just 'Documents,
and the full_original_filename method was returning just '"C:\Documents'
(including the initial double-quotes, but not including the single
quotes), when someone uploaded a file from under their 'C:\Documents and
Settings\' path.

I switched to Ruby-1.8.6-p111 with the security patches posted above,
and the problem went away.

Guys

I need some tutorial on Ruby. It seems to be very
interesting package. advise what do i do so that i
become an expert? am already good at MS Access,
FrontPage, DreamWeaver and a bit of DotNetNuke.

Guys

I need some tutorial on Ruby. It seems to be very
interesting package. advise what do i do so that i
become an expert? am already good at MS Access,
FrontPage, DreamWeaver and a bit of DotNetNuke.

You're not fully correct. All the relevant changes were in array.c
and string.c sources, I've backported both.

I'm not aware of other security problems in the code.

I'll check the link later.

···

On Mon, 23 Jun 2008 19:20:00 +0900 Igal Koshevoy <igal@pragmaticraft.com> mentioned: > Ollivier Robert wrote:

> Try this instead:
> http://www.freebsd.org/cgi/cvsweb.cgi/ports/lang/ruby18/files/

Thanks for the assistance. That FreeBSD web site's UI sucks. Their "Get
diffs" button is broken and always returns nothing. To get a diff on a
file, one must click the "text" next to the revision number.

FreeBSD's backported patch seems insufficient and vulnerable. I come to
this conclusion because they only modified two files (sprintf.c and
string.c) -- but the Ruby changelog for this fix mentions other files
(e.g., array.c), and Zed Shaw identifies about a dozen files potentially
involved in the fix at
http://www.zedshaw.com/rants/the_big_ruby_vulnerabilities.html

--
Stanislav Sedov
ST4096-RIPE

On Mon, 23 Jun 2008 19:20:00 +0900
Igal Koshevoy <igal@pragmaticraft.com> mentioned:

Thanks for the assistance. That FreeBSD web site's UI sucks. Their "Get
diffs" button is broken and always returns nothing. To get a diff on a
file, one must click the "text" next to the revision number.

You rocks! The file you trying to get has only a single revision, and
you obviously requesting the diff between the 1.1 and 1.1 version - that's
empty of course. It's better to look at the text fields before pressing
the button and claiming it doesn't work - isn't it?

···

--
Stanislav Sedov
ST4096-RIPE

So is my patch set now complete, or is there still something missing? I
took a look at eval.c but the changes don't look like security fixes to
me, at first glance.

···

--
Posted via http://www.ruby-forum.com/.

On Mon, 23 Jun 2008 23:27:08 +0900
Igal Koshevoy <igal@pragmaticraft.com> mentioned:
Sorry for the silly mistake, it's been a long

N/p, rest a bit:-)

···

--
Stanislav Sedov
ST4096-RIPE

It's great watching this come together. Thanks to Stanislav and Hongli's
code, and assistance from the rest of you, I think we're getting close
to having a reasonable unofficial patch ready.

I've contacted the following groups and asked them to join the
discussion, and you've already seen some of them join in:
- Ruby Core
- Ruby on Rails blog
- Ruby on Rails core
- RubyInside blog
- Phusion blog
- FreeBSD Ruby maintainer
- Debian Ruby maintainers
- Fedora maintainers
- Portland Ruby Brigade :stuck_out_tongue:

Are there any other persons or groups that can lend a hand that should
be contacted?

If you can think anyone, please ask them to join the ruby-talk mailing
list or use the online thread at http://www.ruby-forum.com/topic/157034

Thanks!

-igal

···

--
Posted via http://www.ruby-forum.com/.

Igal Koshevoy wrote:

As far as I can tell, both of these patches provide a working version of
MRI Ruby. Stanislav and Hongli's p111 backport relies on them having
correctly cherry-picked the right fixes. Smartleaf's p230 fix relies on
the rest of that Ruby version to be correct, and I have some concerns
about other lurking bugs if they shipped a version with such an obvious
flaw.

How do we choose between these?

Well, if we want the Japanese team to be more rigorous in applying test
suites in testing their stuff... we've got the test suites too. That
said, coverage is probably less than perfect, and the silence is
somewhat troublesome to me too.

(BTW, in an earlier note, you asked for someone with C expertise to
audit my p230_fixit_patch.txt. Well, it should be fairly easy to verify
that it reverts class.c to an earlier --- and apparently less broken ---
state; it's quite literally the output of "svn diff". Figuring out what
went wrong with the change it reverts is more difficult. It requires
not
only knowledge of C, but also knowledge of the macros and data
structures
of pre-1.9 MRI. And while I'm pretty good with C, I think, I'm not
nearly
so good with MRI internals. I found the problem not by auditing the
code,
but by doing a simple bisection search of the ruby_1_8_6 branch of the
main svn repository, looking for the first revision that blew up
"rake test").

Robert Thau
rst AT {alum,ai}.mit.edu

···

--
Posted via http://www.ruby-forum.com/\.

Igal Koshevoy wrote:

So before we discard the 17222 patch too casually ... can anyone explain
what Matz was trying to do there? He clearly went through a fair amount
of effort to try to copy those crefs, but why? And does anything else
depend on that change?

Well, as noted earlier, really understanding this code requires
a fairly deep grounding in the internal data structures of MRI
1.8, which I haven't got.

But, for what little it's worth...

The code affected by the 17222 patch seems to mostly have to do
with the internal implementation details of "initialize_copy"
methods on Module and Class, and the cloning of an object's
singleton class when the object itself is cloned. The newer,
post-17222 versions seem to copy more... or try to. More,
perhaps, if I've got the time to investigate the details of
MRI 1.8 representations of method tables...

Robert Thau
rst AT {ai,alum}.mit.edu

···

--
Posted via http://www.ruby-forum.com/\.

Could somebody please explain how to apply the Smartleaf Stanislav and
Hongli's patches? I too am experiencing glibc segmentation problems and
would like to fix my local copy of rails 1.8.6.

Thanks!

Cheri

···

--
Posted via http://www.ruby-forum.com/.

Igal Koshevoy wrote:

As far as I can tell, both of these patches provide a working version of
MRI Ruby. Stanislav and Hongli's p111 backport relies on them having
correctly cherry-picked the right fixes. Smartleaf's p230 fix relies on
the rest of that Ruby version to be correct, and I have some concerns
about other lurking bugs if they shipped a version with such an obvious
flaw.

How do we choose between these?

-igal

Our production Rails environment has been having some memory problems
for a while. Once a week or our mongrel daemons swell in memory usage
and it pushes beyond what our box can comfortably handle. We're hunting
for the root cause, but a simple restart of the mongrel processes clears
it up for another week or so.

We installed the smartleaf patched ruby on our production environment on
Monday. Since that time we had our processes balloon at least a dozen
times in two days. After reverting the patch to ruby it has returned to
normal. I don't know if our situation is a corner case or not. I'm
just wanting to let folks know who are trying to decide what to do that
we've had problems with the smartleaf patch.

···

--
Posted via http://www.ruby-forum.com/\.

Hi,

My thanks to all for such great work. I'm trying to figure out from the
msgs which is the right patch for ruby_1.8.6.p111

It seems that it would be

Posted by Hongli Lai on 23.06.2008 15:40

I've made an updated patch set:
http://blog.phusion.nl/assets/r8ee-security-patch-20080623-2.txt

But when I run it using patch against ruby-1.8.6-p111, I get the
following:
[code]
$ patch < ../ruby_1.8.6.p111.ee-security-patch-20080623-2.txt
patching file array.c
patching file bignum.c
patching file eval.c
patching file intern.h
patching file io.c
can't find file to patch at input line 207
Perhaps you should have used the -p or --strip option?
The text leading up to this was:

···

--------------------------

diff --git a/lib/webrick/httpservlet/filehandler.rb b/lib/webrick/httpservlet/filehandler.rb
index 410cc6f..c8278b7 100644
--- a/lib/webrick/httpservlet/filehandler.rb
+++ b/lib/webrick/httpservlet/filehandler.rb

--------------------------
File to patch: EOF [I entered cntrl-d at this point]
Skip this patch? [y] y
Skipping patch.
4 out of 4 hunks ignored
patching file sprintf.c
patching file string.c
$
[/code]

Advice would be much appreciated.

Thanks and regards,

Larry in New York City
--
Posted via http://www.ruby-forum.com/\.