[Mongrel] [SECURITY] Must Fix This Now! (Re: Arbitrary system files readable in 1.0.4 - 1.1.2)
evan at cloudbur.st
Sat Dec 29 03:31:21 EST 2007
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.
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
More information about the Mongrel-users