4 Replies Latest reply on Oct 13, 2007 6:51 AM by doballve

    Injection threat on EntityQuery order

    doballve Newbie

      While using framework EntityQuery we came up with some extensions that might be worth sharing.. the 1st is a security addition: the 'order' parameter gets directly concatenaded to the the query.. that would allow anything to get injected in the query, possibly resulting in a security threat - yes, it is not SQL, its HQL, but still, you do not want people messing up with your query, do you?

      What we do is to limit the values that can be passed as 'order', like this:

       /**
       * Protect against SQL injection in the order parameter... it gets concatenated to SQL query!
       *
       * @param order String w/ 1 property path + 'ASC'/'DESC'
       * @throws IllegalArgumentException
       * if property path is not among ACCEPTABLE_ORDERBY_PARAMETERS
       */
       @Override
       public void setOrder(String order) {
       // validate parameter
       if (StringUtils.isNotBlank(order)) {
       String[] parts = order.trim().split("\\s", 2);
       parts[0] = StringUtils.trim(parts[0]);
       parts[1] = StringUtils.trim(parts[1]);
       boolean valid = true;
       List<String> acceptable = getAcceptableOrderByParameters();
       valid &= acceptable.contains(parts[0]);
       valid &= ("ASC".equalsIgnoreCase(parts[1]) || "DESC".equalsIgnoreCase(parts[1]));
       if (!valid) {
       throw new IllegalArgumentException("order: " + order);
       }
       } else {
       // blank = null
       order = null;
       }
      
       super.setOrder(order);
       }
      
       protected abstract List<String> getAcceptableOrderByParameters();
      


      and then in the implementing class, defining something like:
       public final static List<String> ACCEPTABLE_ORDERBY_PARAMETERS = Arrays.asList(new String[] {
       "meeting.id",
       "meeting.type",
       "meeting.status",
       "meeting.timestamp",
       });
      
       @Override
       protected List<String> getAcceptableOrderByParameters() {
       return ACCEPTABLE_ORDERBY_PARAMETERS;
       }
      


      In our case we felt it was a good thing to require the abstract method to be implemented. A less radical approach would be to provide a default method returning null and thus accepting any 'order', but letting people restrict it if they want to be safer..Consider it for future version of EntityQuery.

      Diego