Confusing Query.get() behaviour. Possible insufficient criterion checking.

[Former user]

(original reporter: enomad) Also I cant find any documentation about that. I think it can be security breach. Here is the code

https://gist.github.com/stanislav88/8924167

[Mike Bayer]

please explain fully why you think this bug is a security breach. No illegal value is being injected here, the filter() criterion is being ignored and you're getting id #2, just as you would if the filter() criterion hadn't been used. It is of course a bug.

[Mike Bayer]

backported to 0.8

99717570ff085ac2cda16d96155d0b206b5fe713 0.8

2a25e42c08e7541fa70dfc5e6b1ac843d6d02027 0.9

[Former user]

(original author: enomad) >please explain fully why you think this bug is a security breach It can be security breach... when we want to check ownership of related item by something like

item = Item.query.join(User).filter(User == current_user).filter(Item.id == requested_item).one()
if not item: 
    raise 404

and think of get(id) is sugar of filter_by(id=id).one()

[Mike Bayer]

a security breach requires that an attacker can inject a specific, incorrect outcome into code that otherwise performs its function correctly when this injection is not taking place.

In this case, there is no incorrect result that can be returned; get() does its job consistently. It's just that a particular exception related to how get() was used may or may not be raised, and in that sense it behaves more like a query that is inconsistently available, but not in any way one that occasionally returns unexpected results versus expected results. The results it returns, when it's not raising an exception, are consistent and are not subject to manipulation.

A number is passed to get() - the primary key of an object. That number is always the primary key that will be searched. There is no way for the query to return the "wrong" result, as far as get() - get() always returns the object with the pk that you give it, end of story. There is no attack vector by which an attacker can manipulate input into a system such that a different object is returned, and especially not a specific, different object.

An application that writes out session.query(User).filter(User.password == somepassword).get(5), is broken. However, while putting that code in production is a security hazard, the hazard is because the code is entirely broken 100% of the time, not because it can be manipulated in some circumstances to return unexpected results. That particular query will fail every time to check the user's password, which will be revealed in any test. It's not any different from a query that fails to check the password at all.

[Mike Bayer]

Removing milestone: 0.8.xx (automated comment)