[jbossseam-issues] [JBoss JIRA] Commented: (JBSEAM-2099) Support protection against SQL injection in Query order parameter

[Felix Ho?feld (JIRA)]

    [ http://jira.jboss.com/jira/browse/JBSEAM-2099?page=comments#action_12382332 ] 
            
Felix Ho?feld commented on JBSEAM-2099:
---------------------------------------

IMHO this will not do. I may want to have more complex order clauses containing more than one property or even properties of child objects like:

SELECT e FROM Employee e ORDER BY e.department.name, e.lastname, e.firstname

I think this will be a common use case. If the order by statement is not defined by a single property name the patch will not work. Besides it requires that the query object is extended, so for any users using an xml declaration (and I think this is the vast majority) it will be useless. Finally it breaks backward compatibility: It would simply blow up all existing applications that use the order by parameter even if it is hardcoded into the xml and not an el expression (and that are therefore not even threatened).

As an alternative I suggest running the order  against a simple regex, e.g.

if (! order.matches("^[\\w\\s\\.,]$")) 
           throw new IllegalArgumentException("Invalid order by clause in hql statement: " + order);

I don't know HQL syntax well enough: In SQL this would still allow appending queries using UNION but AFAIK UNION is not supported by HQL or JPQL.

BTW I think this bug definitely warants a backport of the final patch and an "official" advisory.




> Support protection against SQL injection in Query order parameter
> -----------------------------------------------------------------
>
>                 Key: JBSEAM-2099
>                 URL: http://jira.jboss.com/jira/browse/JBSEAM-2099
>             Project: JBoss Seam
>          Issue Type: Patch
>          Components: Framework
>    Affects Versions: 2.0.0.CR2
>            Reporter: Diego Ballve
>         Attachments: Query.diff
>
>
> From http://www.jboss.com/index.html?module=bb&op=viewtopic&t=119810
> The 'order' parameter gets directly concatenaded to the query.. that would allow anything to get injected in the query, possibly resulting in a security threat. This patch gives the developer extending framework Query the chance to limit the acceptable order properties.

-- 
This message is automatically generated by JIRA.
-
If you think it was sent incorrectly contact one of the administrators: http://jira.jboss.com/jira/secure/Administrators.jspa
-
For more information on JIRA, see: http://www.atlassian.com/software/jira