[Mongrel] [SECURITY] Must Fix This Now! (Re: Arbitrary system files readable in 1.0.4 - 1.1.2)

Evan Weaver evan at cloudbur.st
Sat Dec 29 03:58:01 EST 2007


Also, I want to reiterate that people should subscribe to the RSS
feed, where handy release announcements come directly to you.

http://mongrel.rubyforge.org/rss.xml

Evan

On Dec 29, 2007 3:31 AM, Evan Weaver <evan at cloudbur.st> wrote:
> New gems are out. You can downgrade to 1.0.3 or you can upgrade to
> 1.0.5 or 1.1.3. Versions prior to 1.0.4 are not affected.
>
> Thanks,
>
> Evan
>
>
> On Dec 28, 2007 9:22 PM, Zed A. Shaw <zedshaw at zedshaw.com> wrote:
> > On Fri, 28 Dec 2007 19:28:25 -0500
> > "Evan Weaver" <evan at cloudbur.st> wrote:
> >
> > > I guess expand_path doesn't interact well with HTTP escaping.
> > >
> > > This is pretty critical, can you file a ticket against it?
> >
> > No, you're miss-reading the change set.  The 1.0.4 change *removed* the
> > expand path on one of the conditions, so now it's using a relative
> > path on one of the if branches, AND removed the check that ensured the
> > expanded_path began with the expanded web root. Notice mine has
> > expand_path on the if and the else so it's always done, and then makes
> > sure that the expanded path begins with the web root.
> >
> > No matter what you do, you *must* expand path all paths before you do
> > any comparisons or reads and never use an indirect path.  It might be
> > better to setup any paths you're doing, and then the very last thing is
> > always expand path.
> >
> > Here's the change again:
> >
> > -      req_path = HttpRequest.unescape(path_info)
> > -      if @path
> > -        req_path = File.expand_path(File.join(@path, path_info), @path)
> > -      else
> > -        req_path = File.expand_path(req_path)
> > -      end
> > -
> > -      if req_path.index(@path) == 0 and File.exist? req_path
> > -        # it exists and it's in the right location
> > +      # Add the drive letter or root path
> > +      req_path = File.join(@path, req_path) if @path
> > +      req_path = File.expand_path req_path
> > +
> > +      if File.exist? req_path
> > +        # It exists and it's in the right location
> >          if File.directory? req_path
> >
> > Notice the - lines have "if req_path.index(@path) == 0..." that's the
> > part that ensures that the given path (after expansion) begins with the
> > path being used by the web server as the root.  If you don't have the
> > required expand path before this, AND make sure that the beginning of
> > the expanded path is always the root path, then you have this bug.
> >
> > I haven't looked at the real code yet, but don't wait for a patch, fix
> > this and I'd say remove the gem so it doesn't go out more since it's a
> > *huge* vulnerability.
> >
> > In fact, pushing out a 1.0.5 that reverts this change to fix it and
> > doing it now is probably the best.
> >
> > --
> > Zed A. Shaw
> > - Hate: http://savingtheinternetwithhate.com/
> > - Good: http://www.zedshaw.com/
> > - Evil: http://yearofevil.com/
> > _______________________________________________
> > Mongrel-users mailing list
> > Mongrel-users at rubyforge.org
> > http://rubyforge.org/mailman/listinfo/mongrel-users
> >
>
>
>
> --
> Evan Weaver
> Cloudburst, LLC
>



-- 
Evan Weaver
Cloudburst, LLC


More information about the Mongrel-users mailing list