On Thu, Apr 14, 2011 at 11: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?
Model.find_by_something really means "find 1 random entry that matches"
What I do is to introduce a "single" method that raises an error if more
than 1 entry is present in an array or relation.
=> Model.find_all_by_something.single (will return nil, 1 instance of
Model, or raise an exception)
A nicer implementation, analog to find_all_by could be:
Model.find_single_by_something
#=> or returns nil
#=> or returns 1 Model instance
#=> or raises an Exception ("FoundMultiple")
Regarding implementation and performance, LIMIT 2 is enough to know
that the result is not "single" and still limiting the performance hit.
HTH,
Peter
--
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.
