On Mon, 23 Jun 2008 22:38:40 +0900
Hongli L. [email protected] mentioned:
Now that you mention it, Keita Y. 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.
eval.c doesn’t pose a security fix as safe_level isn’t secure by design.
It’s just a couple of checks around some functions, nothing more. The
patch
adds another one in eval.c
bignum.c fixes an integer overflow at some operations - this can’t cause
security problems as I could see. It worth applying, though, thanks for
info.
webrick patches isn’t relevant to freebsd in any way, since it fixes
a well known security holes in webrick on windows. These holes were
worked out a while ago (in fact several month or so).
Igal K. wrote:
Thanks for the quick response and for publishing the patch. However, are
you sure you got all the files? Your patch is the most comprehensive
I’ve seen, but isn’t it missing the fixes to things like eval.c, file.c
and bignum.c?
Now that you mention it, Keita Y. 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.
I’ve made an updated patch set:
http://blog.phusion.nl/assets/r8ee-security-patch-20080623-2.txt
Was file.c vulnerable? I see a number of Windows fixes for file.c, but
it’s not immediately clear whether the changes also include security
fixes.
As far as I can tell, you and Stas at FreeBSD were patching different
files. E.g., you patched io.c, while he didn’t seem to. However, I feel
like I don’t understand how to use the FreeBSD website because I can
only see find his patches to string.c and sprintf.c, but none of the
others, so if someone can explain how to find the rest, that’d be great.
I grabbed the patches from the FreeBSD ports tree. Here’s a tarball with
all the patches in FreeBSD’s ruby18 port:
http://blog.phusion.nl/assets/freebsd-ruby18-patches.tar.gz
I excluded some irrelevant (i.e. FreeBSD-specific) patches from my patch
set.
PS: And many thanks for the awesome work on Phusion Passenger and Ruby
EE.
Thanks. 
Stanislav S. 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. 
-igal
On Mon, 23 Jun 2008 23:27:08 +0900
Igal K. [email protected] mentioned:
Sorry for the silly mistake, it’s been a long
N/p, rest a bit:-)
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.
Thomas H. 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.
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 S. 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
Thomas H. wrote:
FreeBSD backported the relevent patches to 1.8.6 p111, perhaps use
those? I’ve certainly not had any problems with my Rails apps with it.
Thanks for the information, Thomas. Could you or someone else with
FreeBSD, as a favor, run the Rails and RSpec test suites with this new
version to determine how well these modified versions work?
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.
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?
Does anybody have access to the CVE details? Selecting patches based on
the CVEs should be easier than guessing based on patches.
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
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 Ruby 1.9.0/1.8.7/1.8.6/1.8.5 new releases (Security Fix) - Ruby - Ruby-Forum
Thanks!
-igal
Igal K. wrote:
All versions of MRI Ruby that claim to fix the vulnerabilities are
either failing with segmentation faults or change the API in ways that
make it impossible to run vital libraries such as Rails 2.0.x and RSpec.
These broken versions include: 1.8.5p231, 1.8.6p230, 1.8.7p22, and
1.9.0-2.
FWIW, I managed to get 1.8.6p230 all the way through a Rails 2.0
app test suite without segfaults or glibc “corrupted memory”
complaints with the patch here:
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).
As always, your milage may vary — but I’m hoping this helps
someone with more detailed knowledge of MRI innards figure out
what’s going on.
Robert Thau
rst AT {ai,alum}.mit.edu
Igal K. wrote:
Thanks!
-igal
I’ve posted some links to the Gentoo Linux Bugzilla
(225465 – dev-lang/ruby <1.8.6_p287 Multiple vulnerabilities (CVE-2008-{1447,2662,2663,2664,2725,2726,2376,3655,3656,3657,3905})). I think they’ll be the
first distro to actually send out a fix.
Igal K. wrote:
All versions of MRI Ruby that claim to fix the vulnerabilities are
either failing with segmentation faults or change the API in ways that
make it impossible to run vital libraries such as Rails 2.0.x and RSpec.
It looks like a fix for the segmentation faults was committed on 21 June
(revision 17530 in the ruby_1_8 branch):
http://svn.ruby-lang.org/cgi-bin/viewvc.cgi?view=rev&revision=17530
Note that this change is only in the ruby_1_8 branch. It hasn’t been
applied to the separate branches for 1.8.5, 1.8.6 and 1.8.7.
I’ve applied the change to 1.8.6-p230 and I’m no longer getting the
segmentation faults in my Rails app. I haven’t tested the change with
1.8.5 or 1.8.7.
The patch I applied to 1.8.6-p230 is available at:
http://files.philross.co.uk/ruby/ruby-1.8.6-p230-fix.patch
This just consists of revision 17530 with the change to ChangeLog
adjusted to apply cleanly.
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):
http://svn.ruby-lang.org/cgi-bin/viewvc.cgi?view=rev&revision=17530
Note that this change is only in the ruby_1_8 branch. It hasn’t been
applied to the separate branches for 1.8.5, 1.8.6 and 1.8.7.
I’ve applied the change to 1.8.6-p230 and I’m no longer getting the
segmentation faults in my Rails app. I haven’t tested the change with
1.8.5 or 1.8.7.
The patch I applied to 1.8.6-p230 is available at:
http://files.philross.co.uk/ruby/ruby-1.8.6-p230-fix.patch
This just consists of revision 17530 with the change to ChangeLog
adjusted to apply cleanly.
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.
Therefore, please consider another solution.
-igal
M. Edward (Ed) Borasky wrote:
I’ve posted some links to the Gentoo Linux Bugzilla
(225465 – dev-lang/ruby <1.8.6_p287 Multiple vulnerabilities (CVE-2008-{1447,2662,2663,2664,2725,2726,2376,3655,3656,3657,3905})).
Thanks for contacting them, seems like there’s some activity over there
as well. I hope they join in the discussion here too.
-igal
We may have a winner!
Can someone with a good understanding of C please audit the patch below?
It seems to make 1.8.6p230 work correctly. It reverts Matz’s “should
copy cref as well” patch, and I’m not clear on what that was intended to
do.
Robert Thau wrote:
FWIW, I managed to get 1.8.6p230 all the way through a Rails 2.0
app test suite without segfaults or glibc “corrupted memory”
complaints with the patch here:
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.
As always, your milage may vary — but I’m hoping this helps
someone with more detailed knowledge of MRI innards figure out
what’s going on.
Same here. Thanks much for posting this!
Does anyone know how to contact the smartleaf folks and get them into
this discussion?
-igal
M. Edward (Ed) Borasky wrote:
Thanks!
-igal
I’ve posted some links to the Gentoo Linux Bugzilla
(225465 – dev-lang/ruby <1.8.6_p287 Multiple vulnerabilities (CVE-2008-{1447,2662,2663,2664,2725,2726,2376,3655,3656,3657,3905})). I think they’ll be the
first distro to actually send out a fix.
How about openSUSE? I just moved the Linux side of my dual-booted laptop
to openSUSE 11.
Igal K. wrote:
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
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 L. wrote:
Now that you mention it, Keita Y. 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 S. 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
Igal K. 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