From 0c966029e2ea68762f2299a816c5629720e0d9d0 Mon Sep 17 00:00:00 2001 From: Sam Brannen <104798+sbrannen@users.noreply.github.com> Date: Mon, 27 Jul 2026 11:12:26 +0300 Subject: [PATCH] Document object design guidelines for SpEL expressions Property accessors resolved via SpEL's ReflectivePropertyAccessor and DataBindingPropertyAccessor may be JavaBean-style accessors or plain accessor methods used to support data classes such as Java records and Kotlin data classes. However, neither accessor can determine, via reflection, whether such a method is a side-effect-free read or an action that happens to return a value. For example, File.delete() is a public method that returns a boolean and therefore looks like a plain "property". To better inform users, this commit updates the Javadoc for ReflectivePropertyAccessor, DataBindingPropertyAccessor, and SimpleEvaluationContext (as well as in the SpEL reference documentation) to clarify that restricting a SimpleEvaluationContext to read-only data binding governs only whether assignment to a property is permitted and does not guarantee that reading a property is free of side effects. The reference documentation's "Security Considerations" section now also defines what makes a method "accessor-shaped", with concrete examples of safe versus side-effecting methods that share that shape (for example, File.delete(), Queue.poll(), and AtomicInteger.incrementAndGet()). Building on that clarification, this commit introduces a new "Object Design" section to the SpEL reference documentation, analogous to the "Model Design" guidance for web data binding. This new section recommends that any object reachable from an untrusted SpEL expression (not only the root object) be a purpose-built, immutable type with a deliberately limited surface area, and that its accessor-shaped methods be audited for unsafe side effects. The new section also notes that reachability is transitive through both property navigation and indexing (for example, rootObject.child.grandchild or rootObject.items[0]). In any case, it remains the responsibility of the code that exposes a root object or other reachable object to an expression from an untrusted source to ensure that none of its accessor-shaped methods perform an unsafe action. Closes gh-37102 --- .../pages/core/expressions/evaluation.adoc | 96 +++++++++++++++++++ .../support/DataBindingPropertyAccessor.java | 10 ++ .../support/ReflectivePropertyAccessor.java | 17 +++- .../spel/support/SimpleEvaluationContext.java | 12 ++- 4 files changed, 131 insertions(+), 4 deletions(-) diff --git a/framework-docs/modules/ROOT/pages/core/expressions/evaluation.adoc b/framework-docs/modules/ROOT/pages/core/expressions/evaluation.adoc index f2ba820702d..58b47491fbf 100644 --- a/framework-docs/modules/ROOT/pages/core/expressions/evaluation.adoc +++ b/framework-docs/modules/ROOT/pages/core/expressions/evaluation.adoc @@ -286,6 +286,102 @@ registering property accessors, resolvers, variables, or functions – to ensure of the objects reachable via the context expose operations that would be dangerous if invoked by an expression from an untrusted source. +Furthermore, a property "getter" reachable from an expression is not necessarily a pure, +side-effect-free read operation. A JavaBean-style accessor (such as `getName()` or +`isActive()`) and a plain accessor method used to support data classes such as Java +records and Kotlin data classes (such as `name()`) are indistinguishable from a method +that performs an action and happens to return a value (that is, a method which is +*accessor-shaped*). For example, the `public boolean delete()` method in `java.io.File` +looks like a plain accessor method to SpEL. Specifically, neither +`ReflectivePropertyAccessor` nor `DataBindingPropertyAccessor` can determine whether such +a method is free of side effects. Moreover, restricting a `SimpleEvaluationContext` to +read-only data binding governs only whether *assignment* to a property is permitted: it +does not verify that reading a property is side-effect-free. When exposing a root object +or other reachable object to an untrusted expression, you must ensure that none of its +accessor-shaped methods perform an action that would be unsafe if triggered by that +expression. + +[NOTE] +.What makes a method "accessor-shaped"? +==== +A method is accessor-shaped if it is `public`, takes no arguments, and returns a value – +the same shape that `ReflectivePropertyAccessor` and `DataBindingPropertyAccessor` look +for when resolving a property "getter" by name. That shape says nothing about whether +invoking the method is actually free of side effects. For example, the following methods +are all accessor-shaped, but only some of them are safe to invoke as a property read. + +Side-effect-free (safe to expose as properties): + +* `getName()` and `isActive()`: conventional JavaBean-style accessors. +* `name()` and `active()`: plain accessor methods used by data classes such as Java + records and Kotlin data classes. + +Side-effecting (unsafe to expose as properties, despite the identical shape): + +* `java.io.File#delete()`: deletes the underlying file and returns whether the deletion + succeeded. +* `java.util.Queue#poll()`: removes and returns the head element, mutating the queue. +* `java.util.concurrent.atomic.AtomicInteger#incrementAndGet()`: increments and returns + a counter, mutating it. + +If an untrusted expression can reference `someFile.delete`, `someQueue.poll`, or +`someCounter.incrementAndGet` as a property, SpEL invokes the corresponding method just +as readily as it would invoke a genuine getter. +==== + +[[expressions-evaluation-context-object-design]] +=== Object Design + +Similar to the design guidance for +xref:web/webmvc/mvc-data-binding.adoc#mvc-data-binding-design[web data binding], you +should carefully design any object that may be reached from a SpEL expression evaluated +against untrusted input. This applies not only to the root object supplied to an +`EvaluationContext` but also to every object that such an expression can navigate to from +that root object – for example, an object returned by a property, a method, an index +operation, a variable, or a function. + +When exposing an object to expressions from an untrusted source, consider the following +recommendations. + +Use a dedicated type:: + Prefer a dedicated type, designed specifically to be evaluated against untrusted + expressions, over passing an existing domain or infrastructure type "as is". A + dedicated type lets you control exactly which properties and methods are reachable from + an expression, rather than exposing the full surface area of a class such as a JPA + entity, `java.io.File`, or a JDBC `Connection` – most of which were never designed with + SpEL evaluation in mind. + +Prefer immutability:: + An immutable type – for example, a Java record or a Kotlin data class exposing only + `val` properties – rules out property writes and eliminates any concern that a "getter" + might mutate state as a side effect, since there is no mutable state to affect. + Immutability does not, on its own, rule out an accessor-shaped method with an external + side effect (such as a network call or a file system operation), but it removes an + entire class of risk. + +Limit scope:: + Expose only the properties and methods that the expression is expected to use, and + nothing more. Because a `PropertyAccessor` cannot restrict access to specific + properties or methods on a per-expression basis, every accessor-shaped method reachable + on an exposed object is reachable by any expression that can reach that object – + regardless of which property or method the application intended the expression to use. + +Audit accessor-shaped methods:: + Review every accessor-shaped method exposed by a type before making it reachable from + an untrusted expression, keeping the <> discussed above in mind. None of the reachable methods should + perform an action that would be unsafe if triggered by that expression. + +[WARNING] +==== +These recommendations apply transitively. If the root object exposes a property or method +that returns another object, and an untrusted expression can navigate to it (for example, +`rootObject.child.grandchild`), the nested object is just as reachable as the root object +itself and must meet the same design requirements. The same is true for an object reached +via indexing (for example, `rootObject.items[0]` or `rootObject.items['key']`): whatever +is returned by the index operation is just as reachable as any other nested object. +==== + [[expressions-evaluation-context-lifecycle]] === Lifecycle and Reuse diff --git a/spring-expression/src/main/java/org/springframework/expression/spel/support/DataBindingPropertyAccessor.java b/spring-expression/src/main/java/org/springframework/expression/spel/support/DataBindingPropertyAccessor.java index 79d647509db..30119d90a4e 100644 --- a/spring-expression/src/main/java/org/springframework/expression/spel/support/DataBindingPropertyAccessor.java +++ b/spring-expression/src/main/java/org/springframework/expression/spel/support/DataBindingPropertyAccessor.java @@ -29,6 +29,16 @@ import java.lang.reflect.Method; * resolve technical properties on {@code java.lang.Object} or {@code java.lang.Class}. * For unrestricted resolution, choose {@link ReflectivePropertyAccessor} instead. * + *

WARNING: Configuring this accessor for read-only access — + * for example, via {@link #forReadOnlyAccess()} or + * {@link SimpleEvaluationContext#forReadOnlyDataBinding()} — disallows + * assignment to a property but does not verify that reading a property is + * free of side effects. See the + * Security Considerations section of the Spring Framework reference + * documentation, as well as the class-level documentation for + * {@link ReflectivePropertyAccessor}, for details. + * * @author Juergen Hoeller * @since 4.3.15 * @see #forReadOnlyAccess() diff --git a/spring-expression/src/main/java/org/springframework/expression/spel/support/ReflectivePropertyAccessor.java b/spring-expression/src/main/java/org/springframework/expression/spel/support/ReflectivePropertyAccessor.java index b9e2fcbd077..0b8fb34abc0 100644 --- a/spring-expression/src/main/java/org/springframework/expression/spel/support/ReflectivePropertyAccessor.java +++ b/spring-expression/src/main/java/org/springframework/expression/spel/support/ReflectivePropertyAccessor.java @@ -58,6 +58,19 @@ import org.springframework.util.StringUtils; * *

A property can be referenced through a public getter method (when being read) * or a public setter method (when being written), and also through a public field. + * A getter method may be a JavaBean-style accessor (for example, {@code getName()} + * or {@code isActive()}) or a plain accessor method used to support data classes + * such as Java records and Kotlin data classes (for example, {@code name()}). + * + *

WARNING: This accessor cannot determine, via reflection, whether + * a candidate getter method is a side-effect-free read of an underlying property or an + * action that happens to return a value — for example, {@code File.delete()} + * matches the same shape as a plain accessor method. See the + * Security Considerations and + * Object Design sections of the Spring Framework reference documentation for + * details. * * @author Andy Clement * @author Juergen Hoeller @@ -381,7 +394,9 @@ public class ReflectivePropertyAccessor implements PropertyAccessor { method = findMethodForProperty(methodSuffixes, "is", clazz, mustBeStatic, 0, BOOLEAN_TYPES); if (method == null) { - // Record-style plain accessor method, for example, name() + // Plain accessor method for a data class, for example, a Java + // record component accessor or a Kotlin data class accessor + // such as name() method = findMethodForProperty(new String[] {propertyName}, "", clazz, mustBeStatic, 0, ANY_TYPES); } diff --git a/spring-expression/src/main/java/org/springframework/expression/spel/support/SimpleEvaluationContext.java b/spring-expression/src/main/java/org/springframework/expression/spel/support/SimpleEvaluationContext.java index e621bb38ee2..fed658a2fa4 100644 --- a/spring-expression/src/main/java/org/springframework/expression/spel/support/SimpleEvaluationContext.java +++ b/spring-expression/src/main/java/org/springframework/expression/spel/support/SimpleEvaluationContext.java @@ -92,10 +92,16 @@ import org.springframework.expression.spel.SpelMessage; * the SpEL language to a subset of its features, that restriction is provided on * a best-effort basis and does not guarantee that evaluation of an expression is * safe; {@code SimpleEvaluationContext} must not be considered safe for evaluating - * a SpEL expression obtained from an untrusted source. See the + * a SpEL expression obtained from an untrusted source. This responsibility extends + * to property accessors: restricting a {@code SimpleEvaluationContext} to read-only + * data binding governs only whether assignment to a property is permitted + * and does not verify that reading a property is free of side effects. See the * Security Considerations section of the Spring Framework reference - * documentation for details. + * >Security Considerations and + * Object Design sections of the Spring Framework reference documentation, as well + * as the class-level documentation for {@link ReflectivePropertyAccessor} and + * {@link DataBindingPropertyAccessor}, for details. * *

Because a parsed {@code Expression} may cache accessor and executor state * resolved against a particular {@code EvaluationContext} configuration, a parsed