Unscoped find is not detected when using together with includes

[jarmo]

The following code is not detected by Brakeman as an unscoped find:

class WhateverController < ApplicationController
  def show
    Foo.includes(:bar).find(params[:id])
  end
end

This, however is catched:

class WhateverController < ApplicationController
  def show
    Foo.find(params[:id])
  end
end

I'm running Brakeman like this: bundle exec brakeman --run-all-checks --quiet --exit-on-warn

[presidentbeef]

Hi Jarmo,

Thank you for letting me know. Kind of tricky to find these cases efficiently.

[hannahhoward]

I just encountered the unscoped find warning for the first time, and I'm concerned it's try to use static analysis to solve for something -- namely authorization -- that's way to complex too be solved with static analysis and simple requires people to write authorization logic. For example, if I'm using CanCanCan, this is a perfectly acceptable solution:

@account = Account.find(params[:id])

authorize :read, @account

This will raise a warning even though it solves the direct object reference vulnerability (assuming my auth logic is sound)

while

Account.extant.find(params[:id]) 

Will not raise the warning but still has a direct object access vulnerability assuming there is a /accounts/:id endpoint.

And this makes more sense anyway cause if you're following REST conventions, and not simply assuming a current_user scope on resources

---

[presidentbeef]

Hi Hannah,

That's a valid comment.

However, I would disagree that Brakeman is trying to "solve" the issue. All Brakeman can do is produce warnings about potential problems. In this case, the warning has been useful for some people.

Additionally, the "unscoped find" check is optional (off by default) and always reports at the "weak" confidence level. (And unlike most Brakeman checks, it was provided by the community.)