Note: This is a beta release of Red Hat Bugzilla 5.0. The data contained within is a snapshot of the live data so any changes you make will not be reflected in the production Bugzilla. Also email is disabled so feel free to test any aspect of the site that you want. File any problems you find or give feedback here.
Bug 1104080 - Every SearchQuery is improperly evaluated as unsafe expression.
Summary: Every SearchQuery is improperly evaluated as unsafe expression.
Keywords:
Status: CLOSED CURRENTRELEASE
Alias: None
Product: Red Hat Enterprise Virtualization Manager
Classification: Red Hat
Component: ovirt-engine
Version: 3.5.0
Hardware: Unspecified
OS: Unspecified
unspecified
high
Target Milestone: ---
: 3.5.0
Assignee: Eli Mesika
QA Contact: Martin Mucha
URL:
Whiteboard: infra
Depends On:
Blocks: rhev3.5beta 1156165
TreeView+ depends on / blocked
 
Reported: 2014-06-03 09:02 UTC by Martin Mucha
Modified: 2016-02-10 19:41 UTC (History)
11 users (show)

Fixed In Version: ovirt-engine-3.5.0_beta
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2015-02-17 17:13:52 UTC
oVirt Team: Infra
Target Upstream Version:
mmucha: needinfo-


Attachments (Terms of Use)


Links
System ID Priority Status Summary Last Updated
oVirt gerrit 28343 master MERGED core,restapi: removed separator inconsistency.. Never

Description Martin Mucha 2014-06-03 09:02:46 UTC
Description of problem:
*Every* SearchQuery is improperly evaluated as unsafe expression.

Version-Release number of selected component (if applicable):
3.5

How reproducible:
100%


Steps to Reproduce:
1. call GET on http://localhost:8080/api/vms or any other such 'getter' method and follow through mentioned methods.


Additional info:
in method
org.ovirt.engine.core.bll.SearchQuery#initQueryData
is this line:
isSafe = SearchObjects.isSafeExpression(searchText);
this line is currently always false, because internally it consists of searching through Map which is filled with <some_uppercase_constant> transformed to lowercase and then concatenated with colon. Example "vms:", "macpool:". The searching code looks like this:
<map>.contains(<input>.toLowerCase());

but that input is calculated here:
org.ovirt.engine.api.restapi.resource.AbstractBackendCollectionResource#getBackendCollection(org.ovirt.engine.core.common.interfaces.SearchType)
using code:
QueryHelper.getConstraint(getUriInfo(), "",  modelType)
which instead of previously mentioned examples produces "vms : ", "macpool : ". Reason is in incompatibilities among:
org.ovirt.engine.api.common.util.QueryHelper#RETURN_TYPE_SEPARTOR
and 
org/ovirt/engine/core/searchbackend/SearchObjects.java:63 //another definition of separator.

Comment: trying to solve sql injection is very improper, related code is extremely inferior and all of this should be thrown away as a whole.

Comment 1 Martin Mucha 2014-06-04 07:44:39 UTC
in
http://gerrit.ovirt.org/#/c/28343/
I've selected " : " to be the right suffix — it seemed to me, that this value is more important for the system, since ':' was only used in determining 'query safeness'. So I've changed ':' to " : ". If it should be the other case, please let me know.

Comment 2 Petr Beňas 2014-08-20 10:30:25 UTC
Is there any way how to test it? Increasing log level or something like that? I'm looking for a way how to confirm the change is in.

Comment 3 Martin Mucha 2014-08-28 13:53:02 UTC
I did a quick scan through code and I do not think this is testable in a black-box way. 
Internals seems too complex to me, so we cannot prove it's ok just upon changing logging level or some other externally observable change like this. Which is, on top of that, not present in code. I.e. I did not find such 'property', but even if it exist somewhere in code, based on it's complexity I do not think it's safe to pronounce it "ok" based just on such observation.
Bug was actually caused by two duplicate constants which weren't in sync, so fix was rather easy.

Comment 4 Eyal Edri 2015-02-17 17:13:52 UTC
rhev 3.5.0 was released. closing.


Note You need to log in before you can comment on or make changes to this bug.