I agree. The optimistic `LIMIT 1` approach to this sort of thing has always been a concern for me (and is the reason I rarely use those dynamic finders).
On Thu, Apr 14, 2011 at 2:15 AM, Corin Langosch <[email protected]>wrote: > Hi, > > while at a security review we noticed the following design/ security flaw > in rails/ activerecord's named finders. > > Named finders (ex. find_by_confirmation_token) are used when no or exactly > one result is desired. Consider the following code: > > account = Account.find_by_confirmation_token(params[:secret_token]) > > Confirmed accounts have their confirmation_token set to nil. Unconfirmed > accounts have some digest there. Now when the controller action is called > without secret_token set in the params hash a random confirmed account is > returned. This is because there's no check if the underlying query returns > more than one row. Of course this is not desired. > > To help finding those subtile bugs/ design flaws, I'd suggest to remove the > "LIMIT 1" from the generated sql by the named finders and instead raise an > exception if the query returns more than one row. Suggestion: "named finders > expect the sql to return at most one result, got xx". > > This change should not have any backwards compatibility issues because a > named finder with a query which selects more than one row is never desired > and a sign for bad design. > > What do you think? > > Corin > > -- > You received this message because you are subscribed to the Google Groups > "Ruby on Rails: Core" group. > To post to this group, send email to [email protected]. > To unsubscribe from this group, send email to > [email protected]. > For more options, visit this group at > http://groups.google.com/group/rubyonrails-core?hl=en. > > -- You received this message because you are subscribed to the Google Groups "Ruby on Rails: Core" group. To post to this group, send email to [email protected]. To unsubscribe from this group, send email to [email protected]. For more options, visit this group at http://groups.google.com/group/rubyonrails-core?hl=en.
