Ruby on Rails | Screencasts | Download | Documentation | Weblog | Community | Source

Ticket #5408 (closed defect: fixed)

Opened 3 years ago

Last modified 3 years ago

Unhandled urls can cause loading of arbitrary ruby files

Reported by: nkriege@hotmail.com Assigned to: Ulysses
Priority: high Milestone: 1.1
Component: ActionPack Version: 1.1.1
Severity: blocker Keywords: ActionController traverse_to_controller
Cc:

Description (Last modified by bitsweat)

In rails 1.1.2, the code in ActionController::Routing::ControllerComponent.traverse_to_controller can cause arbitrary files in your path to be loaded. This happens when the second call to "eval" within that method tries to evaluate a module name that does not correspond to a loaded namespace but does match a file in your load path.

Repro steps

What happens

When you shut down you will see output from the ruby profiler. This is because profile.rb was loaded as a result of the web request.

This is the callstack to the code that loads the arbitrary file: 

ruby/gems/1.8/gems/activesupport-1.3.1/lib/active_support/dependencies.rb:140:in `load'
ruby/gems/1.8/gems/activesupport-1.3.1/lib/active_support/dependencies.rb:140:in `load'
ruby/gems/1.8/gems/activesupport-1.3.1/lib/active_support/dependencies.rb:56:in `require_or_load'
ruby/gems/1.8/gems/activesupport-1.3.1/lib/active_support/dependencies.rb:30:in `depend_on'
ruby/gems/1.8/gems/activesupport-1.3.1/lib/active_support/dependencies.rb:85:in `require_dependency'
ruby/gems/1.8/gems/activesupport-1.3.1/lib/active_support/dependencies.rb:85:in `require_dependency'
ruby/gems/1.8/gems/activesupport-1.3.1/lib/active_support/dependencies.rb:98:in `const_missing'
ruby/gems/1.8/gems/activesupport-1.3.1/lib/active_support/dependencies.rb:131:in `const_missing'
ruby/gems/1.8/gems/actionpack-1.12.1/lib/action_controller/routing.rb:249:in `traverse_to_controller'
generated/routing/recognition.rb:3:in `eval'
ruby/gems/1.8/gems/actionpack-1.12.1/lib/action_controller/routing.rb:249:in `traverse_to_controller'
generated/routing/recognition.rb:3:in `recognize_path'
ruby/gems/1.8/gems/actionpack-1.12.1/lib/action_controller/routing.rb:477:in `recognize!'
ruby/gems/1.8/gems/rails-1.1.2/lib/dispatcher.rb:38:in `dispatch'
ruby/gems/1.8/gems/rails-1.1.2/lib/webrick_server.rb:115:in `handle_dispatch'
ruby/gems/1.8/gems/rails-1.1.2/lib/webrick_server.rb:81:in `service'
ruby/1.8/webrick/httpserver.rb:104:in `service'
ruby/1.8/webrick/httpserver.rb:65:in `run'
ruby/1.8/webrick/server.rb:158:in `start_thread'
ruby/1.8/webrick/server.rb:147:in `start'
ruby/1.8/webrick/server.rb:147:in `start_thread'
ruby/1.8/webrick/server.rb:94:in `start'
ruby/1.8/webrick/server.rb:89:in `each'
ruby/1.8/webrick/server.rb:89:in `start'
ruby/1.8/webrick/server.rb:79:in `start'
ruby/1.8/webrick/server.rb:79:in `start'
ruby/gems/1.8/gems/rails-1.1.2/lib/webrick_server.rb:67:in `dispatch'
ruby/gems/1.8/gems/rails-1.1.2/lib/commands/servers/webrick.rb:59
ruby/site_ruby/1.8/rubygems/custom_require.rb:18:in `require__'
ruby/site_ruby/1.8/rubygems/custom_require.rb:18:in `require'
ruby/gems/1.8/gems/activesupport-1.3.1/lib/active_support/dependencies.rb:147:in `require'
ruby/gems/1.8/gems/rails-1.1.2/lib/commands/server.rb:30
ruby/site_ruby/1.8/rubygems/custom_require.rb:18:in `require__'
ruby/site_ruby/1.8/rubygems/custom_require.rb:18:in `require'
ruby/gems/1.8/gems/activesupport-1.3.1/lib/active_support/dependencies.rb:147:in `require'
script/server:3

I will attach a unit test that demonstrates this.

Attachments

traversal_unit_test.diff (0.8 kB) - added by nkriege@hotmail.com on 06/16/06 01:54:46.
Slightly convoluted, but hopefully this unit test gets the point across

Change History

06/16/06 01:54:46 changed by nkriege@hotmail.com

  • attachment traversal_unit_test.diff added.

Slightly convoluted, but hopefully this unit test gets the point across

06/16/06 03:21:00 changed by rick

Hey, good catch. We just discovered this on the new Rails blog just this week. Looks like it's been fixed in the big routing rewrite. I added the test case to prove it worked.

06/16/06 03:22:10 changed by rick

  • status changed from new to closed.
  • resolution set to fixed.

(In [4451]) add passing test to make sure unhandled requests don't load unnecessary classes. Closed #5408. [nkriege@hotmail.com]

06/16/06 22:54:34 changed by raytrodd@gmail.com

  • status changed from closed to reopened.
  • resolution deleted.

Rick, Any ideas on how long it will be before this change makes it way to the stable branch? We are running into serious issues related to this bug in our production enviornment. Any chance of a hot fix? I imagine many people are susceptible to this issue.

Thanks

06/17/06 00:10:51 changed by rick

Probably not, the 'fix' is the new routing system and won't make it to the stable branch until 1.2 is released.

06/17/06 00:23:54 changed by rick

Question: do you have a catch-all route at the bottom? I've seen this happen on Mephisto.

06/17/06 00:58:47 changed by nkriege@hotmail.com

The same thing happens with or without a catch-all route. Either way, hitting a url of http://hostname/foo will end up in traverse_to_controller. If you do not have a FooController, then we will try to eval "::Object::Foo". The auto-loading code in Module::const_missing will then load foo.rb, if it exists.

FYI: My workaround for now is to return nil unless mod.const_defined?(mod_name) right before the second call to eval in traverse_to_controller. This breaks accessing controllers nested within modules, though, unless you explicitly set each one of them in routes.rb.

06/17/06 01:17:47 changed by nkriege@hotmail.com

Actually, my last comment may have been misleading. I have only seen this bug when I have a route like ":controller/:action/:id". What I meant was that it does not matter if I have a route like "*name".

06/18/06 02:35:08 changed by drbrain

  • priority changed from high to highest.
  • severity changed from normal to blocker.

This is a major security exploit. You can easily DOS any Rails app with this.

You need to release a bugfix now.

06/18/06 04:58:06 changed by ulysses

[4456]

06/18/06 19:33:08 changed by bitsweat

  • owner changed from David to Ulysses.
  • status changed from reopened to new.
  • description changed.
  • milestone set to 1.1.

06/21/06 16:09:51 changed by fastjames@gmail.com

At present, changeset 4456 appears to break routing to controllers inside of subdirectories. I have /app/controllers/foo/bar_controller.rb, and requests for http://sitename/foo/bar/list now return "Routing Error / Recognition failed for "/foo/bar/list". If I revert vendor/rails to revision 4455 the problem goes away, and routing to the controller works fine. Updating back to the latest svn revision then causes the problem to reappear.

I understand that this change is important, so I'll be happy to open this in a new ticket if you'd prefer.

06/29/06 17:24:51 changed by fastjames@gmail.com

Today's update to branches/stable appears to have fixed this problem. Huzzah!

06/30/06 05:52:12 changed by bitsweat

  • status changed from new to closed.
  • resolution set to fixed.