CVE-2016-6652 (Java, published 2016-10-05T16:59Z)
Description: 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.
Severity: MEDIUM (CVSS v3 base score: 5.6)
Weakness: CWE-89 \u2014 Improper Neutralization of Special Elements used in an SQL Command ('SQL Injection')
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.
Repository: https://github.com/spring-projects/spring-data-jpa
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 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.

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"));
	}