CVE-2016-6652: Improper Neutralization of Special Elements used in an SQL Command ('SQL Injection') (CWE-89)
value: SQL injection vulnerability in Pivotal Spring Data JPA before 1.9.6 (Gosling SR6) and 1.10.x before 1.10.4 (Hopper SR4), when used with a repository that defines a String query using the @Query annotation, allows attackers to execute arbitrary JPQL commands via a sort instance with a function call.
CWE description: The product constructs all or part of an SQL command using externally-influenced input from an upstream component, but it does not neutralize or incorrectly neutralizes special elements that could modify the intended SQL command when it is sent to a downstream component. Without sufficient removal or quoting of SQL syntax in user-controllable inputs, the generated SQL query can cause those inputs to be interpreted as SQL instead of ordinary user data.
CVSS v3 base score: 5.6
Published: 2016-10-05T16:59Z
Repository: https://github.com/spring-projects/spring-data-jpa
Language: Java
Files changed: src/main/asciidoc/jpa.adoc, src/main/java/org/springframework/data/jpa/domain/JpaSort.java, src/main/java/org/springframework/data/jpa/repository/query/QueryUtils.java, src/test/java/org/springframework/data/jpa/domain/JpaSortTests.java, src/test/java/org/springframework/data/jpa/repository/query/QueryUtilsUnitTests.java
Commit date: 2016-09-20 10:28:05 +0200
Commit message: DATAJPA-965 - Fix potential blind SQL injection in Sort when used in combination with @Query.

We now decline sort expressions that contain functions such as ORDER BY LENGTH(name) when used with repository having a String query defined via the @Query annotation.

Think of a query method as follows:

@Query("select p from Person p where LOWER(p.lastname) = LOWER(:lastname)")
List<Person> findByLastname(@Param("lastname") String lastname, Sort sort);

Calls to findByLastname("lannister", new Sort("LENGTH(firstname)")) from now on throw an Exception indicating function calls are not allowed within the _ORDER BY_ clause. However you still can use JpaSort.unsafe("LENGTH(firstname)") to restore the behavior.

Kudos to Niklas Särökaari, Joona Immonen, Arto Santala, Antti Virtanen, Michael Holopainen and Antti Ahola who brought this to our attention.
Diff: diff --git a/src/main/asciidoc/jpa.adoc b/src/main/asciidoc/jpa.adoc
index d4e9d58..c2d2965 100644
--- a/src/main/asciidoc/jpa.adoc
+++ b/src/main/asciidoc/jpa.adoc
@@ -301,10 +301,46 @@ public interface UserRepository extends JpaRepository<User, Long> {
 
 ====
 
 This also works with named native queries by adding the suffix `.count` to a copy of your query. Be aware that you probably must register a result set mapping for your count query, though.
 
+[[jpa.query-methods.sorting]]
+=== Using Sort
+
+Sorting can be done be either providing a `PageRequest` or using `Sort` directly. The properties actually used within the `Order` instances of `Sort` need to match to your domain model, which means they need to resolve to either a property or an alias used within the query. The JPQL defines this as a _state_field_path_expression_.
+
+[NOTE]
+====
+Using any non referenceable path expression leads to an Exception.
+====
+
+Using `Sort` together with <<jpa.query-methods.at-query, @Query>> however allows you to sneak in non path checked `Order` instances containing _functions_ within the `ORDER BY` clause. This is possible because the `Order` is just appended to the given query string. By default we will reject any `Order` instance containing function calls, but you can use `JpaSort.unsafe` to add potentially unsafe ordering.
+
+.Using Sort and JpaSort
+====
+[source, java]
+----
+public interface UserRepository extends JpaRepository<User, Long> {
+
+  @Query("select u from User u where u.lastname like ?1%")
+  List<User> findByAndSort(String lastname, Sort sort);
+
+  @Query("select u.id, LENGTH(u.firstname) as fn_len from User u where u.lastname like ?1%")
+  List<Object[]> findByAsArrayAndSort(String lastname, Sort sort);
+}
+
+repo.findByAndSort("lannister", new Sort("firstname"));               <1>
+repo.findByAndSort("stark", new Sort("LENGTH(firstname)"));           <2>
+repo.findByAndSort("targaryen", JpaSort.unsafe("LENGTH(firstname)")); <3>
+repo.findByAsArrayAndSort("bolton", new Sort("fn_len"));              <4>
+----
+<1> Valid `Sort` expression pointing to property in domain model.
+<2> Invalid `Sort` containing function call. Thows Exception.
+<3> Valid `Sort` containing explicitly _unsafe_ `Order`.
+<4> Valid `Sort` expression pointing to aliased function.
+====
+
 [[jpa.named-parameters]]
 === Using named parameters
 
 By default Spring Data JPA will use position based parameter binding as described in all the samples above. This makes query methods a little error prone to refactoring regarding the parameter position. To solve this issue you can use `@Param` annotation to give a method parameter a concrete name and bind the name in the query.
 
diff --git a/src/main/java/org/springframework/data/jpa/domain/JpaSort.java b/src/main/java/org/springframework/data/jpa/domain/JpaSort.java
index 9bbc136..53fd2e8 100644
--- a/src/main/java/org/springframework/data/jpa/domain/JpaSort.java
+++ b/src/main/java/org/springframework/data/jpa/domain/JpaSort.java
@@ -1,7 +1,7 @@
 /*
- * Copyright 2013-2015 the original author or authors.
+ * Copyright 2013-2016 the original author or authors.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
  * You may obtain a copy of the License at
  *
@@ -30,10 +30,11 @@ import org.springframework.util.Assert;
 /**
  * Sort option for queries that wraps JPA meta-model {@link Attribute}s for sorting.
  * 
  * @author Thomas Darimont
  * @author Oliver Gierke
+ * @author Christoph Strobl
  */
 public class JpaSort extends Sort {
 
 	private static final long serialVersionUID = 1L;
 
@@ -81,10 +82,14 @@ public class JpaSort extends Sort {
 
 	private JpaSort(List<Order> orders, Direction direction, List<Path<?, ?>> paths) {
 		super(combine(orders, direction, paths));
 	}
 
+	private JpaSort(List<Order> orders) {
+		super(orders);
+	}
+
 	/**
 	 * Returns a new {@link JpaSort} with the given sorting criteria added to the current one.
 	 * 
 	 * @param direction can be {@literal null}.
 	 * @param attributes must not be {@literal null}.
@@ -115,10 +120,34 @@ public class JpaSort extends Sort {
 		}
 
 		return new JpaSort(existing, direction, Arrays.asList(paths));
 	}
 
+	/**
+	 * Returns a new {@link JpaSort} with the given sorting criteria added to the current one.
+	 *
+	 * @param direction can be {@literal null}.
+	 * @param properties must not be {@literal null} or empty.
+	 * @return
+	 */
+	public JpaSort andUnsafe(Direction direction, String... properties) {
+
+		Assert.notEmpty(properties, "Properties must not be null!");
+
+		List<Order> orders = new ArrayList<Order>();
+
+		for (Order order : this) {
+			orders.add(order);
+		}
+
+		for (String property : properties) {
+			orders.add(new JpaOrder(direction, property));
+		}
+
+		return new JpaSort(orders, direction, Collections.<Path<?, ?>> emptyList());
+	}
+
 	/**
 	 * Turns the given {@link Attribute}s into {@link Path}s.
 	 * 
 	 * @param attributes must not be {@literal null} or empty.
 	 * @return
@@ -172,10 +201,55 @@ public class JpaSort extends Sort {
 
 		Assert.notNull(attribute, "Attribute must not be null!");
 		return new Path<T, S>(Arrays.asList(attribute));
 	}
 
+	/**
+	 * Creates new unsafe {@link JpaSort} based on given properties.
+	 *
+	 * @param properties must not be {@literal null} or empty.
+	 * @return
+	 */
+	public static JpaSort unsafe(String... properties) {
+		return unsafe(Sort.DEFAULT_DIRECTION, properties);
+	}
+
+	/**
+	 * Creates new unsafe {@link JpaSort} based on given {@link Direction} and properties.
+	 *
+	 * @param direction must not be {@literal null}.
+	 * @param properties must not be {@literal null} or empty.
+	 * @return
+	 */
+	public static JpaSort unsafe(Direction direction, String... properties) {
+
+		Assert.notNull(direction, "Direction must not be null!");
+		Assert.notEmpty(properties, "Properties must not be empty!");
+		Assert.noNullElements(properties, "Properties must not contain null values!");
+
+		return unsafe(direction, Arrays.asList(properties));
+	}
+
+	/**
+	 * Creates new unsafe {@link JpaSort} based on given {@link Direction} and properties.
+	 *
+	 * @param direction must not be {@literal null}.
+	 * @param properties must not be {@literal null} or empty.
+	 * @return
+	 */
+	public static JpaSort unsafe(Direction direction, List<String> properties) {
+
+		Assert.notEmpty(properties, "Properties must not be empty!");
+
+		List<Order> orders = new ArrayList<Order>();
+		for (String property : properties) {
+			orders.add(new JpaOrder(direction, property));
+		}
+
+		return new JpaSort(orders);
+	}
+
 	/**
 	 * Value object to abstract a collection of {@link Attribute}s.
 	 * 
 	 * @author Oliver Gierke
 	 */
@@ -231,6 +305,133 @@ public class JpaSort extends Sort {
 			}
 
 			return builder.length() == 0 ? "" : builder.substring(0, builder.lastIndexOf("."));
 		}
 	}
+
+	/**
+	 * @author Christoph Strobl
+	 */
+	public static class JpaOrder extends Order {
+
+		private final boolean unsafe;
+		private final boolean ignoreCase;
+
+		/**
+		 * Creates a new {@link JpaOrder} instance. if order is {@literal null} then order defaults to
+		 * {@link Sort#DEFAULT_DIRECTION}
+		 *
+		 * @param direction can be {@literal null}, will default to {@link Sort#DEFAULT_DIRECTION}.
+		 * @param property must not be {@literal null}.
+		 */
+		private JpaOrder(Direction direction, String property) {
+			this(direction, property, NullHandling.NATIVE);
+		}
+
+		/**
+		 * Creates a new {@link Order} instance. if order is {@literal null} then order defaults to
+		 * {@link Sort#DEFAULT_DIRECTION}.
+		 *
+		 * @param direction can be {@literal null}, will default to {@link Sort#DEFAULT_DIRECTION}.
+		 * @param property must not be {@literal null}.
+		 * @param nullHandlingHint can be {@literal null}, will default to {@link NullHandling#NATIVE}.
+		 */
+		private JpaOrder(Direction direction, String property, NullHandling nullHandlingHint) {
+			this(direction, property, nullHandlingHint, false, true);
+		}
+
+		private JpaOrder(Direction direction, String property, NullHandling nullHandling, boolean ignoreCase,
+				boolean unsafe) {
+
+			super(direction, property, nullHandling);
+			this.ignoreCase = ignoreCase;
+			this.unsafe = unsafe;
+		}
+
+		/*
+		 * (non-Javadoc)
+		 * @see org.springframework.data.domain.Sort.Order#with(org.springframework.data.domain.Sort.Direction)
+		 */
+		@Override
+		public JpaOrder with(Direction order) {
+			return new JpaOrder(order, getProperty(), getNullHandling(), isIgnoreCase(), this.unsafe);
+		}
+
+		/*
+		 * (non-Javadoc)
+		 * @see org.springframework.data.domain.Sort.Order#with(org.springframework.data.domain.Sort.NullHandling)
+		 */
+		@Override
+		public JpaOrder with(NullHandling nullHandling) {
+			return new JpaOrder(getDirection(), getProperty(), nullHandling, isIgnoreCase(), this.unsafe);
+		}
+
+		/*
+		 * (non-Javadoc)
+		 * @see org.springframework.data.domain.Sort.Order#nullsFirst()
+		 */
+		@Override
+		public JpaOrder nullsFirst() {
+			return with(NullHandling.NULLS_FIRST);
+		}
+
+		/*
+		 * (non-Javadoc)
+		 * @see org.springframework.data.domain.Sort.Order#nullsLast()
+		 */
+		@Override
+		public JpaOrder nullsLast() {
+			return with(NullHandling.NULLS_LAST);
+		}
+
+		/*
+		 * (non-Javadoc)
+		 * @see org.springframework.data.domain.Sort.Order#nullsNative()
+		 */
+		public JpaOrder nullsNative() {
+			return with(NullHandling.NATIVE);
+		}
+
+		/**
+		 * Creates new {@link Sort} with potentially unsafe {@link Order} instances.
+		 *
+		 * @param properties must not be {@literal null}.
+		 * @return
+		 */
+		public Sort withUnsafe(String... properties) {
+
+			Assert.notEmpty(properties, "Properties must not be empty!");
+			Assert.noNullElements(properties, "Properties must not contain null values!");
+
+			List<Order> orders = new ArrayList<Order>();
+			for (String property : properties) {
+				orders.add(new JpaOrder(getDirection(), property, getNullHandling(), isIgnoreCase(), this.unsafe));
+			}
+			return new Sort(orders);
+		}
+
+		/*
+		 * (non-Javadoc)
+		 * @see org.springframework.data.domain.Sort.Order#ignoreCase()
+		 */
+		@Override
+		public JpaOrder ignoreCase() {
+			return new JpaOrder(getDirection(), getProperty(), getNullHandling(), true, this.unsafe);
+		}
+
+		/*
+		 * (non-Javadoc)
+		 * @see org.springframework.data.domain.Sort.Order#isIgnoreCase()
+		 */
+		@Override
+		public boolean isIgnoreCase() {
+			return super.isIgnoreCase() || ignoreCase;
+		}
+
+		/**
+		 * @return true if {@link JpaOrder} created {@link #withUnsafe(String...)}.
+		 */
+		public boolean isUnsafe() {
+			return unsafe;
+		}
+	}
 }
diff --git a/src/main/java/org/springframework/data/jpa/repository/query/QueryUtils.java b/src/main/java/org/springframework/data/jpa/repository/query/QueryUtils.java
index 3825b10..f40c042 100644
--- a/src/main/java/org/springframework/data/jpa/repository/query/QueryUtils.java
+++ b/src/main/java/org/springframework/data/jpa/repository/query/QueryUtils.java
@@ -51,12 +51,14 @@ import javax.persistence.metamodel.Attribute.PersistentAttributeType;
 import javax.persistence.metamodel.Bindable;
 import javax.persistence.metamodel.ManagedType;
 import javax.persistence.metamodel.PluralAttribute;
 
 import org.springframework.core.annotation.AnnotationUtils;
+import org.springframework.dao.InvalidDataAccessApiUsageException;
 import org.springframework.data.domain.Sort;
 import org.springframework.data.domain.Sort.Order;
+import org.springframework.data.jpa.domain.JpaSort.JpaOrder;
 import org.springframework.data.mapping.PropertyPath;
 import org.springframework.util.Assert;
 import org.springframework.util.StringUtils;
 
 /**
@@ -64,10 +66,11 @@ import org.springframework.util.StringUtils;
  * 
  * @author Oliver Gierke
  * @author Kevin Raymond
  * @author Thomas Darimont
  * @author Komi Innocent
+ * @author Christoph Strobl
  */
 public abstract class QueryUtils {
 
 	public static final String COUNT_QUERY_STRING = "select count(%s) from %s x";
 	public static final String DELETE_ALL_QUERY_STRING = "delete from %s x";
@@ -100,10 +103,14 @@ public abstract class QueryUtils {
 	private static final Map<PersistentAttributeType, Class<? extends Annotation>> ASSOCIATION_TYPES;
 
 	private static final int QUERY_JOIN_ALIAS_GROUP_INDEX = 2;
 	private static final int VARIABLE_NAME_GROUP_INDEX = 4;
 
+	private static final Pattern PUNCTATION_PATTERN = Pattern.compile(".*((?![\\._])[\\p{Punct}|\\s])");
+	private static final String FUNCTION_ALIAS_GROUP_NAME = "alias";
+	private static final Pattern FUNCTION_PATTERN;
+
 	static {
 
 		StringBuilder builder = new StringBuilder();
 		builder.append("(?<=from)"); // from as starting delimiter
 		builder.append("(?:\\s)+"); // at least one space separating
@@ -143,10 +150,17 @@ public abstract class QueryUtils {
 		builder.append("\\(");
 		builder.append(".*");
 		builder.append("\\)");
 
 		CONSTRUCTOR_EXPRESSION = compile(builder.toString(), CASE_INSENSITIVE + DOTALL);
+
+		builder = new StringBuilder();
+		builder.append("\\s+"); // at least one space
+		builder.append("\\w+\\([0-9a-zA-z\\._,\\s']+\\)"); // any function call including parameters within the brackets
+		builder.append("\\s+[as|AS]+\\s+(?<" + FUNCTION_ALIAS_GROUP_NAME + ">[\\w\\.]+)"); // the potential alias
+
+		FUNCTION_PATTERN = compile(builder.toString());
 	}
 
 	/**
 	 * Private constructor to prevent instantiation.
 	 */
@@ -225,13 +239,14 @@ public abstract class QueryUtils {
 		} else {
 			builder.append(", ");
 		}
 
 		Set<String> aliases = getOuterJoinAliases(query);
+		Set<String> functionAliases = getFunctionAliases(query);
 
 		for (Order order : sort) {
-			builder.append(getOrderClause(aliases, alias, order)).append(", ");
+			builder.append(getOrderClause(aliases, functionAliases, alias, order)).append(", ");
 		}
 
 		builder.delete(builder.length() - 2, builder.length());
 
 		return builder.toString();
@@ -244,13 +259,20 @@ public abstract class QueryUtils {
 	 * @param joinAliases the join aliases of the original query.
 	 * @param alias the alias for the root entity.
 	 * @param order the order object to build the clause for.
 	 * @return
 	 */
-	private static String getOrderClause(Set<String> joinAliases, String alias, Order order) {
+	private static String getOrderClause(Set<String> joinAliases, Set<String> functionAlias, String alias, Order order) {
 
 		String property = order.getProperty();
+
+		checkSortExpression(order);
+
+		if (functionAlias.contains(property)) {
+			return String.format("%s %s", property, toJpaDirection(order));
+		}
+
 		boolean qualifyReference = !property.contains("("); // ( indicates a function
 
 		for (String joinAlias : joinAliases) {
 			if (property.startsWith(joinAlias.concat("."))) {
 				qualifyReference = false;
@@ -285,10 +307,32 @@ public abstract class QueryUtils {
 		}
 
 		return result;
 	}
 
+	/**
+	 * Returns the aliases used for aggregate functions like {@code SUM, COUNT, ...}.
+	 *
+	 * @param query
+	 * @return
+	 */
+	static Set<String> getFunctionAliases(String query) {
+
+		Set<String> result = new HashSet<String>();
+		Matcher matcher = FUNCTION_PATTERN.matcher(query);
+
+		while (matcher.find()) {
+
+			String alias = matcher.group(FUNCTION_ALIAS_GROUP_NAME);
+			if (StringUtils.hasText(alias)) {
+				result.add(alias);
+			}
+		}
+
+		return result;
+	}
+
 	private static String toJpaDirection(Order order) {
 		return order.getDirection().name().toLowerCase(Locale.US);
 	}
 
 	/**
@@ -615,6 +659,25 @@ public abstract class QueryUtils {
 			}
 		}
 
 		return false;
 	}
+
+	/**
+	 * Check any given {@link JpaOrder#isUnsafe()} order for presence of at least one property offending the
+	 * {@link #PUNCTATION_PATTERN} and throw an {@link Exception} indicating potential unsafe order by expression.
+	 *
+	 * @param order
+	 */
+	private static void checkSortExpression(Order order) {
+
+		if (order instanceof JpaOrder && ((JpaOrder) order).isUnsafe()) {
+			return;
+		}
+
+		if (PUNCTATION_PATTERN.matcher(order.getProperty()).find()) {
+			throw new InvalidDataAccessApiUsageException(String
+					.format("Sort expression '%s' must not contain functions or expressions. Please use JpaSort.unsafe.", order));
+		}
+	}
+
 }
diff --git a/src/test/java/org/springframework/data/jpa/domain/JpaSortTests.java b/src/test/java/org/springframework/data/jpa/domain/JpaSortTests.java
index 56db573..49d28e1 100644
--- a/src/test/java/org/springframework/data/jpa/domain/JpaSortTests.java
+++ b/src/test/java/org/springframework/data/jpa/domain/JpaSortTests.java
@@ -1,7 +1,7 @@
 /*
- * Copyright 2013-2015 the original author or authors.
+ * Copyright 2013-2016 the original author or authors.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
  * You may obtain a copy of the License at
  *
@@ -27,10 +27,11 @@ import javax.persistence.metamodel.PluralAttribute;
 import org.junit.Test;
 import org.junit.runner.RunWith;
 import org.springframework.data.domain.Sort;
 import org.springframework.data.domain.Sort.Direction;
 import org.springframework.data.domain.Sort.Order;
+import org.springframework.data.jpa.domain.JpaSort.JpaOrder;
 import org.springframework.data.jpa.domain.JpaSort.Path;
 import org.springframework.data.jpa.domain.sample.Address_;
 import org.springframework.data.jpa.domain.sample.MailMessage_;
 import org.springframework.data.jpa.domain.sample.MailSender_;
 import org.springframework.data.jpa.domain.sample.Role_;
@@ -45,10 +46,11 @@ import org.springframework.test.context.junit4.SpringJUnit4ClassRunner;
  * bootstrapped.
  * 
  * @see DATAJPA-12
  * @author Thomas Darimont
  * @author Oliver Gierke
+ * @author Christoph Strobl
  */
 @RunWith(SpringJUnit4ClassRunner.class)
 @ContextConfiguration("classpath:infrastructure.xml")
 public class JpaSortTests {
 
@@ -171,6 +173,58 @@ public class JpaSortTests {
 	public void buildsUpPathForPluralAttributesCorrectly() {
 
 		assertThat(new JpaSort(path(User_.colleagues).dot(User_.roles).dot(Role_.name)), //
 				hasItem(new Order(ASC, "colleagues.roles.name")));
 	}
+
+	/**
+	 * @see DATAJPA-???
+	 */
+	@Test
+	public void createsUnsafeSortCorrectly() {
+
+		JpaSort sort = JpaSort.unsafe(DESC, "foo.bar");
+
+		assertThat(sort, hasItem(new Order(DESC, "foo.bar")));
+		assertThat(sort.getOrderFor("foo.bar"), is(instanceOf(JpaOrder.class)));
+	}
+
+	/**
+	 * @see DATAJPA-???
+	 */
+	@Test
+	public void createsUnsafeSortWithMultiplePropertiesCorrectly() {
+
+		JpaSort sort = JpaSort.unsafe(DESC, "foo.bar", "spring.data");
+
+		assertThat(sort, hasItems(new Order(DESC, "foo.bar"), new Order(DESC, "spring.data")));
+		assertThat(sort.getOrderFor("foo.bar"), is(instanceOf(JpaOrder.class)));
+		assertThat(sort.getOrderFor("spring.data"), is(instanceOf(JpaOrder.class)));
+	}
+
+	/**
+	 * @see DATAJPA-???
+	 */
+	@Test
+	public void combinesSafeAndUnsafeSortCorrectly() {
+
+		JpaSort sort = new JpaSort(path(User_.colleagues).dot(User_.roles).dot(Role_.name)).andUnsafe(DESC, "foo.bar");
+
+		assertThat(sort, hasItems(new Order(ASC, "colleagues.roles.name"), new Order(DESC, "foo.bar")));
+		assertThat(sort.getOrderFor("colleagues.roles.name"), is(not(instanceOf(JpaOrder.class))));
+		assertThat(sort.getOrderFor("foo.bar"), is(instanceOf(JpaOrder.class)));
+	}
+
+	/**
+	 * @see DATAJPA-???
+	 */
+	@Test
+	public void combinesUnsafeAndSafeSortCorrectly() {
+
+		Sort sort = JpaSort.unsafe(DESC, "foo.bar").and(ASC, path(User_.colleagues).dot(User_.roles).dot(Role_.name));
+
+		assertThat(sort, hasItems(new Order(ASC, "colleagues.roles.name"), new Order(DESC, "foo.bar")));
+		assertThat(sort.getOrderFor("colleagues.roles.name"), is(not(instanceOf(JpaOrder.class))));
+		assertThat(sort.getOrderFor("foo.bar"), is(instanceOf(JpaOrder.class)));
+	}
+
 }
diff --git a/src/test/java/org/springframework/data/jpa/repository/query/QueryUtilsUnitTests.java b/src/test/java/org/springframework/data/jpa/repository/query/QueryUtilsUnitTests.java
index 30ca4f2..7d4f233 100644
--- a/src/test/java/org/springframework/data/jpa/repository/query/QueryUtilsUnitTests.java
+++ b/src/test/java/org/springframework/data/jpa/repository/query/QueryUtilsUnitTests.java
@@ -1,7 +1,7 @@
 /*
- * Copyright 2008-2015 the original author or authors.
+ * Copyright 2008-2016 the original author or authors.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
  * You may obtain a copy of the License at
  *
@@ -21,18 +21,21 @@ import static org.springframework.data.jpa.repository.query.QueryUtils.*;
 
 import java.util.Set;
 
 import org.hamcrest.Matcher;
 import org.junit.Test;
+import org.springframework.dao.InvalidDataAccessApiUsageException;
 import org.springframework.data.domain.Sort;
+import org.springframework.data.jpa.domain.JpaSort;
 
 /**
  * Unit test for {@link QueryUtils}.
  * 
  * @author Oliver Gierke
  * @author Thomas Darimont
  * @author Komi Innocent
+ * @author Christoph Strobl
  */
 public class QueryUtilsUnitTests {
 
 	static final String QUERY = "select u from User u";
 	static final String FQ_QUERY = "select u from org.acme.domain.User$Foo_Bar u";
@@ -228,11 +231,11 @@ public class QueryUtilsUnitTests {
 	}
 
 	/**
 	 * @see DATAJPA-148
 	 */
-	@Test
+	@Test(expected = InvalidDataAccessApiUsageException.class)
 	public void doesNotPrefixSortsIfFunction() {
 
 		Sort sort = new Sort("sum(foo)");
 		assertThat(applySorting("select p from Person p", sort, "p"), endsWith("order by sum(foo) asc"));
 	}
@@ -359,9 +362,137 @@ public class QueryUtilsUnitTests {
 	public void doesNotQualifySortIfNoAliasDetected() {
 		assertThat(applySorting("from mytable where ?1 is null", new Sort("firstname")),
 				endsWith("order by firstname asc"));
 	}
 
+	/**
+	 * @see DATAJPA-???
+	 */
+	@Test(expected = InvalidDataAccessApiUsageException.class)
+	public void doesNotAllowWhitespaceInSort() {
+
+		Sort sort = new Sort("case when foo then bar");
+		applySorting("select p from Person p", sort, "p");
+	}
+
+	/**
+	 * @see DATAJPA-???
+	 */
+	@Test
+	public void doesNotPrefixUnsageJpaSortFunctionCalls() {
+
+		JpaSort sort = JpaSort.unsafe("sum(foo)");
+		assertThat(applySorting("select p from Person p", sort, "p"), endsWith("order by sum(foo) asc"));
+	}
+
+	/**
+	 * @see DATAJPA-???
+	 */
+	@Test
+	public void doesNotPrefixMultipleAliasedFunctionCalls() {
+
+		String query = "SELECT AVG(m.price) AS avgPrice, SUM(m.stocks) AS sumStocks FROM Magazine m";
+		Sort sort = new Sort("avgPrice", "sumStocks");
+
+		assertThat(applySorting(query, sort, "m"), endsWith("order by avgPrice asc, sumStocks asc"));
+	}
+
+	/**
+	 * @see DATAJPA-???
+	 */
+	@Test
+	public void doesNotPrefixSingleAliasedFunctionCalls() {
+
+		String query = "SELECT AVG(m.price) AS avgPrice FROM Magazine m";
+		Sort sort = new Sort("avgPrice");
+
+		assertThat(applySorting(query, sort, "m"), endsWith("order by avgPrice asc"));
+	}
+
+	/**
+	 * @see DATAJPA-???
+	 */
+	@Test
+	public void prefixesSingleNonAliasedFunctionCallRelatedSortProperty() {
+
+		String query = "SELECT AVG(m.price) AS avgPrice FROM Magazine m";
+		Sort sort = new Sort("someOtherProperty");
+
+		assertThat(applySorting(query, sort, "m"), endsWith("order by m.someOtherProperty asc"));
+	}
+
+	/**
+	 * @see DATAJPA-???
+	 */
+	@Test
+	public void prefixesNonAliasedFunctionCallRelatedSortPropertyWhenSelectClauseContainesAliasedFunctionForDifferentProperty() {
+
+		String query = "SELECT m.name, AVG(m.price) AS avgPrice FROM Magazine m";
+		Sort sort = new Sort("name", "avgPrice");
+
+		assertThat(applySorting(query, sort, "m"), endsWith("order by m.name asc, avgPrice asc"));
+	}
+
+	/**
+	 * @see DATAJPA-???
+	 */
+	@Test
+	public void doesNotPrefixAliasedFunctionCallNameWithMultipleNumericParameters() {
+
+		String query = "SELECT SUBSTRING(m.name, 2, 5) AS trimmedName FROM Magazine m";
+		Sort sort = new Sort("trimmedName");
+
+		assertThat(applySorting(query, sort, "m"), endsWith("order by trimmedName asc"));
+	}
+
+	/**
+	 * @see DATAJPA-???
+	 */
+	@Test
+	public void doesNotPrefixAliasedFunctionCallNameWithMultipleStringParameters() {
+
+		String query = "SELECT CONCAT(m.name, 'foo') AS extendedName FROM Magazine m";
+		Sort sort = new Sort("extendedName");
+
+		assertThat(applySorting(query, sort, "m"), endsWith("order by extendedName asc"));
+	}
+
+	/**
+	 * @see DATAJPA-???
+	 */
+	@Test
+	public void doesNotPrefixAliasedFunctionCallNameWithUnderscores() {
+
+		String query = "SELECT AVG(m.price) AS avg_price FROM Magazine m";
+		Sort sort = new Sort("avg_price");
+
+		assertThat(applySorting(query, sort, "m"), endsWith("order by avg_price asc"));
+	}
+
+	/**
+	 * @see DATAJPA-???
+	 */
+	@Test
+	public void doesNotPrefixAliasedFunctionCallNameWithDots() {
+
+		String query = "SELECT AVG(m.price) AS m.avg FROM Magazine m";
+		Sort sort = new Sort("m.avg");
+
+		assertThat(applySorting(query, sort, "m"), endsWith("order by m.avg asc"));
+	}
+
+	/**
+	 * @see DATAJPA-???
+	 */
+	@Test
+	public void doesNotPrefixAliasedFunctionCallNameWhenQueryStringContainsMultipleWhiteSpaces() {
+
+		String query = "SELECT  AVG(  m.price  )   AS   avgPrice   FROM Magazine   m";
+		Sort sort = new Sort("avgPrice");
+
+		assertThat(applySorting(query, sort, "m"), endsWith("order by avgPrice asc"));
+	}
+
 	private static void assertCountQuery(String originalQuery, String countQuery) {
 		assertThat(createCountQueryFor(originalQuery), is(countQuery));
 	}
 }

Vulnerable code:  * Copyright 2013-2015 the original author or authors.
			builder.append(getOrderClause(aliases, alias, order)).append(", ");
	private static String getOrderClause(Set<String> joinAliases, String alias, Order order) {
 * Copyright 2013-2015 the original author or authors.
 * Copyright 2008-2015 the original author or authors.
	@Test
Fixed code: [[jpa.query-methods.sorting]]
=== Using Sort

Sorting can be done be either providing a `PageRequest` or using `Sort` directly. The properties actually used within the `Order` instances of `Sort` need to match to your domain model, which means they need to resolve to either a property or an alias used within the query. The JPQL defines this as a _state_field_path_expression_.

[NOTE]
====
Using any non referenceable path expression leads to an Exception.
====

Using `Sort` together with <<jpa.query-methods.at-query, @Query>> however allows you to sneak in non path checked `Order` instances containing _functions_ within the `ORDER BY` clause. This is possible because the `Order` is just appended to the given query string. By default we will reject any `Order` instance containing function calls, but you can use `JpaSort.unsafe` to add potentially unsafe ordering.

.Using Sort and JpaSort
====
[source, java]
----
public interface UserRepository extends JpaRepository<User, Long> {

  @Query("select u from User u where u.lastname like ?1%")
  List<User> findByAndSort(String lastname, Sort sort);

  @Query("select u.id, LENGTH(u.firstname) as fn_len from User u where u.lastname like ?1%")
  List<Object[]> findByAsArrayAndSort(String lastname, Sort sort);
}

repo.findByAndSort("lannister", new Sort("firstname"));               <1>
repo.findByAndSort("stark", new Sort("LENGTH(firstname)"));           <2>
repo.findByAndSort("targaryen", JpaSort.unsafe("LENGTH(firstname)")); <3>
repo.findByAsArrayAndSort("bolton", new Sort("fn_len"));              <4>
----
<1> Valid `Sort` expression pointing to property in domain model.
<2> Invalid `Sort` containing function call. Thows Exception.
<3> Valid `Sort` containing explicitly _unsafe_ `Order`.
<4> Valid `Sort` expression pointing to aliased function.
====

 * Copyright 2013-2016 the original author or authors.
 * @author Christoph Strobl
	private JpaSort(List<Order> orders) {
		super(orders);
	}

	/**
	 * Returns a new {@link JpaSort} with the given sorting criteria added to the current one.
	 *
	 * @param direction can be {@literal null}.
	 * @param properties must not be {@literal null} or empty.
	 * @return
	 */
	public JpaSort andUnsafe(Direction direction, String... properties) {

		Assert.notEmpty(properties, "Properties must not be null!");

		List<Order> orders = new ArrayList<Order>();

		for (Order order : this) {
			orders.add(order);
		}

		for (String property : properties) {
			orders.add(new JpaOrder(direction, property));
		}

		return new JpaSort(orders, direction, Collections.<Path<?, ?>> emptyList());
	}

	/**
	 * Creates new unsafe {@link JpaSort} based on given properties.
	 *
	 * @param properties must not be {@literal null} or empty.
	 * @return
	 */
	public static JpaSort unsafe(String... properties) {
		return unsafe(Sort.DEFAULT_DIRECTION, properties);
	}

	/**
	 * Creates new unsafe {@link JpaSort} based on given {@link Direction} and properties.
	 *
	 * @param direction must not be {@literal null}.
	 * @param properties must not be {@literal null} or empty.
	 * @return
	 */
	public static JpaSort unsafe(Direction direction, String... properties) {

		Assert.notNull(direction, "Direction must not be null!");
		Assert.notEmpty(properties, "Properties must not be empty!");
		Assert.noNullElements(properties, "Properties must not contain null values!");

		return unsafe(direction, Arrays.asList(properties));
	}

	/**
	 * Creates new unsafe {@link JpaSort} based on given {@link Direction} and properties.
	 *
	 * @param direction must not be {@literal null}.
	 * @param properties must not be {@literal null} or empty.
	 * @return
	 */
	public static JpaSort unsafe(Direction direction, List<String> properties) {

		Assert.notEmpty(properties, "Properties must not be empty!");

		List<Order> orders = new ArrayList<Order>();
		for (String property : properties) {
			orders.add(new JpaOrder(direction, property));
		}

		return new JpaSort(orders);
	}


	/**
	 * @author Christoph Strobl
	 */
	public static class JpaOrder extends Order {

		private final boolean unsafe;
		private final boolean ignoreCase;

		/**
		 * Creates a new {@link JpaOrder} instance. if order is {@literal null} then order defaults to
		 * {@link Sort#DEFAULT_DIRECTION}
		 *
		 * @param direction can be {@literal null}, will default to {@link Sort#DEFAULT_DIRECTION}.
		 * @param property must not be {@literal null}.
		 */
		private JpaOrder(Direction direction, String property) {
			this(direction, property, NullHandling.NATIVE);
		}

		/**
		 * Creates a new {@link Order} instance. if order is {@literal null} then order defaults to
		 * {@link Sort#DEFAULT_DIRECTION}.
		 *
		 * @param direction can be {@literal null}, will default to {@link Sort#DEFAULT_DIRECTION}.
		 * @param property must not be {@literal null}.
		 * @param nullHandlingHint can be {@literal null}, will default to {@link NullHandling#NATIVE}.
		 */
		private JpaOrder(Direction direction, String property, NullHandling nullHandlingHint) {
			this(direction, property, nullHandlingHint, false, true);
		}

		private JpaOrder(Direction direction, String property, NullHandling nullHandling, boolean ignoreCase,
				boolean unsafe) {

			super(direction, property, nullHandling);
			this.ignoreCase = ignoreCase;
			this.unsafe = unsafe;
		}

		/*
		 * (non-Javadoc)
		 * @see org.springframework.data.domain.Sort.Order#with(org.springframework.data.domain.Sort.Direction)
		 */
		@Override
		public JpaOrder with(Direction order) {
			return new JpaOrder(order, getProperty(), getNullHandling(), isIgnoreCase(), this.unsafe);
		}

		/*
		 * (non-Javadoc)
		 * @see org.springframework.data.domain.Sort.Order#with(org.springframework.data.domain.Sort.NullHandling)
		 */
		@Override
		public JpaOrder with(NullHandling nullHandling) {
			return new JpaOrder(getDirection(), getProperty(), nullHandling, isIgnoreCase(), this.unsafe);
		}

		/*
		 * (non-Javadoc)
		 * @see org.springframework.data.domain.Sort.Order#nullsFirst()
		 */
		@Override
		public JpaOrder nullsFirst() {
			return with(NullHandling.NULLS_FIRST);
		}

		/*
		 * (non-Javadoc)
		 * @see org.springframework.data.domain.Sort.Order#nullsLast()
		 */
		@Override
		public JpaOrder nullsLast() {
			return with(NullHandling.NULLS_LAST);
		}

		/*
		 * (non-Javadoc)
		 * @see org.springframework.data.domain.Sort.Order#nullsNative()
		 */
		public JpaOrder nullsNative() {
			return with(NullHandling.NATIVE);
		}

		/**
		 * Creates new {@link Sort} with potentially unsafe {@link Order} instances.
		 *
		 * @param properties must not be {@literal null}.
		 * @return
		 */
		public Sort withUnsafe(String... properties) {

			Assert.notEmpty(properties, "Properties must not be empty!");
			Assert.noNullElements(properties, "Properties must not contain null values!");

			List<Order> orders = new ArrayList<Order>();
			for (String property : properties) {
				orders.add(new JpaOrder(getDirection(), property, getNullHandling(), isIgnoreCase(), this.unsafe));
			}
			return new Sort(orders);
		}

		/*
		 * (non-Javadoc)
		 * @see org.springframework.data.domain.Sort.Order#ignoreCase()
		 */
		@Override
		public JpaOrder ignoreCase() {
			return new JpaOrder(getDirection(), getProperty(), getNullHandling(), true, this.unsafe);
		}

		/*
		 * (non-Javadoc)
		 * @see org.springframework.data.domain.Sort.Order#isIgnoreCase()
		 */
		@Override
		public boolean isIgnoreCase() {
			return super.isIgnoreCase() || ignoreCase;
		}

		/**
		 * @return true if {@link JpaOrder} created {@link #withUnsafe(String...)}.
		 */
		public boolean isUnsafe() {
			return unsafe;
		}
	}
import org.springframework.dao.InvalidDataAccessApiUsageException;
import org.springframework.data.jpa.domain.JpaSort.JpaOrder;
 * @author Christoph Strobl
	private static final Pattern PUNCTATION_PATTERN = Pattern.compile(".*((?![\\._])[\\p{Punct}|\\s])");
	private static final String FUNCTION_ALIAS_GROUP_NAME = "alias";
	private static final Pattern FUNCTION_PATTERN;


		builder = new StringBuilder();
		builder.append("\\s+"); // at least one space
		builder.append("\\w+\\([0-9a-zA-z\\._,\\s']+\\)"); // any function call including parameters within the brackets
		builder.append("\\s+[as|AS]+\\s+(?<" + FUNCTION_ALIAS_GROUP_NAME + ">[\\w\\.]+)"); // the potential alias

		FUNCTION_PATTERN = compile(builder.toString());
		Set<String> functionAliases = getFunctionAliases(query);
			builder.append(getOrderClause(aliases, functionAliases, alias, order)).append(", ");
	private static String getOrderClause(Set<String> joinAliases, Set<String> functionAlias, String alias, Order order) {

		checkSortExpression(order);

		if (functionAlias.contains(property)) {
			return String.format("%s %s", property, toJpaDirection(order));
		}

	/**
	 * Returns the aliases used for aggregate functions like {@code SUM, COUNT, ...}.
	 *
	 * @param query
	 * @return
	 */
	static Set<String> getFunctionAliases(String query) {

		Set<String> result = new HashSet<String>();
		Matcher matcher = FUNCTION_PATTERN.matcher(query);

		while (matcher.find()) {

			String alias = matcher.group(FUNCTION_ALIAS_GROUP_NAME);
			if (StringUtils.hasText(alias)) {
				result.add(alias);
			}
		}

		return result;
	}


	/**
	 * Check any given {@link JpaOrder#isUnsafe()} order for presence of at least one property offending the
	 * {@link #PUNCTATION_PATTERN} and throw an {@link Exception} indicating potential unsafe order by expression.
	 *
	 * @param order
	 */
	private static void checkSortExpression(Order order) {

		if (order instanceof JpaOrder && ((JpaOrder) order).isUnsafe()) {
			return;
		}

		if (PUNCTATION_PATTERN.matcher(order.getProperty()).find()) {
			throw new InvalidDataAccessApiUsageException(String
					.format("Sort expression '%s' must not contain functions or expressions. Please use JpaSort.unsafe.", order));
		}
	}

 * Copyright 2013-2016 the original author or authors.
import org.springframework.data.jpa.domain.JpaSort.JpaOrder;
 * @author Christoph Strobl

	/**
	 * @see DATAJPA-???
	 */
	@Test
	public void createsUnsafeSortCorrectly() {

		JpaSort sort = JpaSort.unsafe(DESC, "foo.bar");

		assertThat(sort, hasItem(new Order(DESC, "foo.bar")));
		assertThat(sort.getOrderFor("foo.bar"), is(instanceOf(JpaOrder.class)));
	}

	/**
	 * @see DATAJPA-???
	 */
	@Test
	public void createsUnsafeSortWithMultiplePropertiesCorrectly() {

		JpaSort sort = JpaSort.unsafe(DESC, "foo.bar", "spring.data");

		assertThat(sort, hasItems(new Order(DESC, "foo.bar"), new Order(DESC, "spring.data")));
		assertThat(sort.getOrderFor("foo.bar"), is(instanceOf(JpaOrder.class)));
		assertThat(sort.getOrderFor("spring.data"), is(instanceOf(JpaOrder.class)));
	}

	/**
	 * @see DATAJPA-???
	 */
	@Test
	public void combinesSafeAndUnsafeSortCorrectly() {

		JpaSort sort = new JpaSort(path(User_.colleagues).dot(User_.roles).dot(Role_.name)).andUnsafe(DESC, "foo.bar");

		assertThat(sort, hasItems(new Order(ASC, "colleagues.roles.name"), new Order(DESC, "foo.bar")));
		assertThat(sort.getOrderFor("colleagues.roles.name"), is(not(instanceOf(JpaOrder.class))));
		assertThat(sort.getOrderFor("foo.bar"), is(instanceOf(JpaOrder.class)));
	}

	/**
	 * @see DATAJPA-???
	 */
	@Test
	public void combinesUnsafeAndSafeSortCorrectly() {

		Sort sort = JpaSort.unsafe(DESC, "foo.bar").and(ASC, path(User_.colleagues).dot(User_.roles).dot(Role_.name));

		assertThat(sort, hasItems(new Order(ASC, "colleagues.roles.name"), new Order(DESC, "foo.bar")));
		assertThat(sort.getOrderFor("colleagues.roles.name"), is(not(instanceOf(JpaOrder.class))));
		assertThat(sort.getOrderFor("foo.bar"), is(instanceOf(JpaOrder.class)));
	}

 * Copyright 2008-2016 the original author or authors.
import org.springframework.dao.InvalidDataAccessApiUsageException;
import org.springframework.data.jpa.domain.JpaSort;
 * @author Christoph Strobl
	@Test(expected = InvalidDataAccessApiUsageException.class)
	/**
	 * @see DATAJPA-???
	 */
	@Test(expected = InvalidDataAccessApiUsageException.class)
	public void doesNotAllowWhitespaceInSort() {

		Sort sort = new Sort("case when foo then bar");
		applySorting("select p from Person p", sort, "p");
	}

	/**
	 * @see DATAJPA-???
	 */
	@Test
	public void doesNotPrefixUnsageJpaSortFunctionCalls() {

		JpaSort sort = JpaSort.unsafe("sum(foo)");
		assertThat(applySorting("select p from Person p", sort, "p"), endsWith("order by sum(foo) asc"));
	}

	/**
	 * @see DATAJPA-???
	 */
	@Test
	public void doesNotPrefixMultipleAliasedFunctionCalls() {

		String query = "SELECT AVG(m.price) AS avgPrice, SUM(m.stocks) AS sumStocks FROM Magazine m";
		Sort sort = new Sort("avgPrice", "sumStocks");

		assertThat(applySorting(query, sort, "m"), endsWith("order by avgPrice asc, sumStocks asc"));
	}

	/**
	 * @see DATAJPA-???
	 */
	@Test
	public void doesNotPrefixSingleAliasedFunctionCalls() {

		String query = "SELECT AVG(m.price) AS avgPrice FROM Magazine m";
		Sort sort = new Sort("avgPrice");

		assertThat(applySorting(query, sort, "m"), endsWith("order by avgPrice asc"));
	}

	/**
	 * @see DATAJPA-???
	 */
	@Test
	public void prefixesSingleNonAliasedFunctionCallRelatedSortProperty() {

		String query = "SELECT AVG(m.price) AS avgPrice FROM Magazine m";
		Sort sort = new Sort("someOtherProperty");

		assertThat(applySorting(query, sort, "m"), endsWith("order by m.someOtherProperty asc"));
	}

	/**
	 * @see DATAJPA-???
	 */
	@Test
	public void prefixesNonAliasedFunctionCallRelatedSortPropertyWhenSelectClauseContainesAliasedFunctionForDifferentProperty() {

		String query = "SELECT m.name, AVG(m.price) AS avgPrice FROM Magazine m";
		Sort sort = new Sort("name", "avgPrice");

		assertThat(applySorting(query, sort, "m"), endsWith("order by m.name asc, avgPrice asc"));
	}

	/**
	 * @see DATAJPA-???
	 */
	@Test
	public void doesNotPrefixAliasedFunctionCallNameWithMultipleNumericParameters() {

		String query = "SELECT SUBSTRING(m.name, 2, 5) AS trimmedName FROM Magazine m";
		Sort sort = new Sort("trimmedName");

		assertThat(applySorting(query, sort, "m"), endsWith("order by trimmedName asc"));
	}

	/**
	 * @see DATAJPA-???
	 */
	@Test
	public void doesNotPrefixAliasedFunctionCallNameWithMultipleStringParameters() {

		String query = "SELECT CONCAT(m.name, 'foo') AS extendedName FROM Magazine m";
		Sort sort = new Sort("extendedName");

		assertThat(applySorting(query, sort, "m"), endsWith("order by extendedName asc"));
	}

	/**
	 * @see DATAJPA-???
	 */
	@Test
	public void doesNotPrefixAliasedFunctionCallNameWithUnderscores() {

		String query = "SELECT AVG(m.price) AS avg_price FROM Magazine m";
		Sort sort = new Sort("avg_price");

		assertThat(applySorting(query, sort, "m"), endsWith("order by avg_price asc"));
	}

	/**
	 * @see DATAJPA-???
	 */
	@Test
	public void doesNotPrefixAliasedFunctionCallNameWithDots() {

		String query = "SELECT AVG(m.price) AS m.avg FROM Magazine m";
		Sort sort = new Sort("m.avg");

		assertThat(applySorting(query, sort, "m"), endsWith("order by m.avg asc"));
	}

	/**
	 * @see DATAJPA-???
	 */
	@Test
	public void doesNotPrefixAliasedFunctionCallNameWhenQueryStringContainsMultipleWhiteSpaces() {

		String query = "SELECT  AVG(  m.price  )   AS   avgPrice   FROM Magazine   m";
		Sort sort = new Sort("avgPrice");

		assertThat(applySorting(query, sort, "m"), endsWith("order by avgPrice asc"));
	}