Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Sneaky Params via Resource Identifier Configuration #30467

Closed
schmijos opened this issue Aug 30, 2017 · 4 comments · Fixed by #35236

Comments

@schmijos
Copy link
Contributor

@schmijos schmijos commented Aug 30, 2017

This is probably a very hypothetical problem. And please don't ask, how I found it. But somehow it still bugs me.

Steps to reproduce

Since Rails 4.0 it is possible to sneak in additional parameters via the param option for resources. You can do something like that:

routes.draw do
  resources :users, param: :'name/:sneaky'
end

ActionDispatch::Routing::Mapper::Resources::Resource#initialize just takes it as-is. Later the strings generated by #member_scope and #nested_param are handed over to the Journey::Parser which creates an AST with the sneaked in parameters.

You will see that the parameters are added to the RouteSet and can be used by the controller action:

def show
  render plain: "#{params[:name]} : #{params[:sneaky]}"
end

Full reproduction example is available here (visualizer can be commented out): https://github.com/renuo/railshoeck-route-mapping/blob/master/sample.rb#L21

Expected behavior

I'd expect the mapper to escape the colon somehow or to throw a route mapping/generation error.

System configuration

Rails version:
4.0 up to master (currently at 87eb1a2)

Ruby version:
Since you can write colons in symbols.

Further questions

  • Is there any possibility that someone out there is using this as a feature?
  • Is there any possibility to sneak in user input into your route generation? Some sort of CMS maybe? Like param: :'id/:is_admin'...
@rails-bot rails-bot bot added the stale label Nov 28, 2017
@rails-bot

This comment has been minimized.

Copy link

@rails-bot rails-bot bot commented Nov 28, 2017

This issue has been automatically marked as stale because it has not been commented on for at least three months.
The resources of the Rails team are limited, and so we are asking for your help.
If you can still reproduce this error on the 5-1-stable branch or on master, please reply with all of the information you have about it in order to keep the issue open.
Thank you for all your contributions.

@rails-bot rails-bot bot closed this Dec 5, 2017
@jeremy

This comment has been minimized.

Copy link
Member

@jeremy jeremy commented Dec 5, 2017

This is def sneaky and surprising. PR welcome to raise ArgumentError when there's a colon in the param.

@rails-bot rails-bot bot removed the stale label Dec 5, 2017
schmijos added a commit to renuo/rails that referenced this issue Feb 12, 2019
Fixes rails#30467. After this change
it's not possible anymore to configure routes like this:

    routes.draw do
      resources :users, param: "name/:sneaky"
    end
schmijos added a commit to renuo/rails that referenced this issue Feb 12, 2019
Fixes rails#30467. After this change
it's not possible anymore to configure routes like this:

    routes.draw do
      resources :users, param: "name/:sneaky"
    end
@schmijos

This comment has been minimized.

Copy link
Contributor Author

@schmijos schmijos commented Feb 13, 2019

@jeremy Fixed by #35236

schmijos added a commit to renuo/rails that referenced this issue Feb 13, 2019
Fixes rails#30467. After this change
it's not possible anymore to configure routes like this:

    routes.draw do
      resources :users, param: "name/:sneaky"
    end
schmijos added a commit to renuo/rails that referenced this issue Feb 13, 2019
Fixes rails#30467. After this change
it's not possible anymore to configure routes like this:

    routes.draw do
      resources :users, param: "name/:sneaky"
    end
schmijos added a commit to renuo/rails that referenced this issue Mar 26, 2019
After this change it's not possible anymore to configure routes
like this:

    routes.draw do
      resources :users, param: "name/:sneaky"
    end

Fixes rails#30467.
rafaelfranca added a commit that referenced this issue Mar 27, 2019
 Prohibit sneaky custom params from being drawn (Fix #30467)
@searls

This comment has been minimized.

Copy link
Contributor

@searls searls commented Sep 23, 2019

Is there any possibility that someone out there is using this as a feature?

For what it's worth, I was actually using this as a feature (upgrading to 6.0 today so just got bit)

  resources :weeks, param: "year/:month/:day"

I was using this for identifying weeks not based on an id, but instead on a params[:year],params[:month],params[:day] tuple. ¯_(ツ)_/¯

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.