searching with _in predicate if the argument is empty returns All records

[tyronewilson]

I don't know if this issue has been brought up before but for me this is unexpected behaviour.

If I use a query something like MyModel.search(parent_id_or_id_in: some_association.pluck(:my_model_id)).result This works fine if I have some_association records. If I don't have any it will return ALL of the MyModel records instead of none.

This is unexpected behaviour in my view and is a serious authorization issue for us as we are basing some user actions and permissions based on the results of these kinds of searches.

If I search for id_in: [] I get ALL records back, I would expect fan empty ActiveRecord::Relation under such circumstances. The same thing happens if I use nil

The following examples outline the problem:

1. SomeModel.search(id_in: []).result #all records returned
2. SomeModel.search(id_in: nil).result #all records returned
3. SomeModel.search(id_in: SomeModel.none).result #all records returned
4. SomeModel.search(id_in: {}).result #returns all records

Some cases which are handled properly

1. SomeModel.search(id_in: 'bla').result #returns empty relation
2. SomeModel.search(id_in: 3).result #returns empty relation

As mentioned before, this has become a bit of a security issue for us and I hope you will see it the same way. I tried to see how one might fix it but not being familiar with such a big project as Ransack I wasn't sure.

Please let me know if we can get this fixed.

[jonatack]

Hi @tyronewilson, you could populate the id search field input with a default value like 0 between the user input and the ransack action, when you'd like to be certain the result will be filtered by triggering a search.

P.S. - I'd recommend using the ransack class method instead of search for the reasons mentioned in the README.

[tyronewilson]

Perhaps I am a bit slow. How does one specify the default value? I saw a trick in another issue where someone adds [-1] to all arrays for querying but I couldn't come up with a reliable way to do that without having a maintenance nightmare.

Perhaps you can provide me with an example of your meaning above @jonatack?

[jonatack]

This seems to be expected behavior (if I'm understanding correctly).

[tyronewilson]

Well if you think so. My reading is that that vulnerability is centred around converting [] to nil automatically. However in both our cases ransack was simply returning ALL records when clearly it doesn't make sense to.

If I have User which has_many :friends and has_many :postings and I want to get all postings from a users friends (and no others). I would perhaps do something like

def friends_postings
   Posting.search(user_id_in: user.friends.pluck(:id)).result
end

This would be fine assuming the user has at least one friend. However, if someone is new to your system, their call to friends_postings would immediately return Posting.all.

That seems like a very serious security issue to me if there is a way I should be writing these queries instead I would like to know it.

I think the fix should be simple. if an _in ransack key is passed an [] or nil or {}, it should return Posting.none since no arguments were passed in essentially.

For now I have created a guard around functions where I do this

def guard(model, with: , &block)
    with.empty? ? model.none : yield
end

to be used like so:

def friends_postings
   guard Posting, with: user.friends do
      Posting.search(user_id_in: user.friends.pluck(:id)).result
   end
end

Looks and feels cumbersome but it works.

[jonatack]

_I think the fix should be simple. if an in ransack key is passed an [] or nil or {}, it should return Posting.none since no arguments were passed in essentially.

If you'd like to make a PR that passes the current tests and does this, and that doesn't trigger filtering when there is no user input, and works well with the :wants_array option, go for it 👍.

[tyronewilson]

Cool I'll give it some thought. Thanks.

On 24 March 2015 at 20:01, Jon Atack notifications@github.com wrote:

"I think the fix should be simple. if an _in ransack key is passed an []
or nil or {}, it should return Posting.none since no arguments were passed
in essentially."

If you'd like to make a PR that passes the current tests and does this,
and that doesn't trigger filtering when there is no user input, go for it [image:
👍]

—
Reply to this email directly or view it on GitHub
#524 (comment)
.

[ideaoforder]

👍

[svey]

Any update on this? appears the behavior hasn't changed. Model.ransack(some_column_in: []) returns Model.all :(

[KarimElsayad247]

I would love if someone could shed some light on why this is expected behavior. At a glance, it seems illogical. Why doesn everything match nothing? How does ransack perform its _in matching?