Programming Thoughts
Struts 2 - Annotation-based Validation
Value Stack is a Security Hazard

Value Stack

At the heart of Struts 2 request processing is the Value Stack and the Object Graph Notation Language used to access it. One is created for every request and is thread local. It comes in two parts, the first is the Object Stack, which is a LIFO stack of the Action, ModelDriven model, and temporary objects, such as loop iterators. The second is the Context Map, which contains various data, such as servlet context, session data, and request parameters. This allows sophisticated presentation logic in JSP and other result processors to access any aspect of the request, whether database records retrieved by the Action to be displayed, conversation state in session, or application configuration.

The Value Stack is also used in request parameter processing where parameter names are interpreted by OGNL to find the property to set. For example, if a parameter is named 'city', OGNL will traverse down the Object Stack till it finds an object with a property of the same name, usually the Action. If named 'address.city', a property named 'address' is found and, if it's an object, its 'city' property is set. This is useful for where an Action has a different form for each processing function or a form is an object tree. This also applies to the model of ModelDriven Actions, which is placed higher on the Object Stack than the Action.

The first problem is the parameter names are set by the user's browser, which he controls, and processing isn't restricted to what's meant to be the form fields. The default Struts workflow is the Action that processes a form also displays the result. If, for example, an Action has a field of the user's authorisations, a malicious user could guess the name, set a parameter name to match it, and set the value. Similarly, properties in the Context Map can also be set, including session data.

Security

Struts 2 does provide security by having the Parameters interceptor ignore parameter names that, according to documentation, match the following regex pattern.

dojo..*,^struts..*,^session..*,^request..*,^application..*,^servlet(Request|Response)..*,parameters...*

The documentation is known to be out of date and the actual pattern is actually multiple patterns, comes from com.opensymphony.xwork2.security.DefaultExcludedPatternsChecker and, for Struts 6.9.0, are the following monstrosities.

(^|\\%\\{)(#?top\\.)[^\\s]* (^|\\%\\{)((#?)(top(\\.|\\['|\\[\")|\\[\\d\\]\\.)?)(dojo|struts|session|request|response|application|servlet(Request|Response|Context)|parameters|context|_memberAccess)(\\.|\\[).* .*(^|\\.|\\[|\\'|\"|get)class(\\(\\.|\\[|\\'|\").* actionErrors|actionMessages|fieldErrors

The above pattern is an accumulation of all the blocks against tricks pulled by hackers, known as OGNL injection, to execute remote code. The Parameters interceptor can be configured with a custom pattern and it's recommended to include the above. A new pattern must be configured if a new OGNL exploit is discovered.

This leads to a another problem that developers can be surprised that form field names are silently ignored. For example, in a Stack Overflow question, a developer was confused why 'application.appName' was not allowed as a form field name.

Explanation of OGNL injection is beyond the scope of this document. Consider the security blog, The Hacker vs. Struts 2 Game – It Appears it has No Ending, for further details.

OGNL Isn't Needed

The identified problems are:

• Malicious users can set properties besides the intended form.
• More OGNL injection exploits may yet be discovered, needing updates to a dense, regex pattern.
• Legitimate form field names can be silently discarded.

The key design flaw is using a complex library to be clever, such as a ModelDriven model inserted on the Object Stack to receive parameters instead of the Action, but it's overengineering. In practice, forms rarely need to be more than the flat key/value pairs of HTTP forms. Scrapping OGNL eliminates a security hazard.

One loss is, in default Struts 2 workflow, an Action can have multiple forms, requiring form field names to specify which form receives the submission, but this workflow is flawed for not following Post/Redirect/Get. Instead, each form should have its own form processing Action, which define the object to receive form data using ModelDriven. Scrapping OGNL forces abandonment of a design that's bad practice in the first place. It also solves the other security hazard. If a request parameter name does not match a form field name, it's discarded and it's obvious why it was.

This may seem like advocating the complete abandonment of OGNL but the problem only occurs with form processing, where values are set by users. When generating web pages, any expressions are set by the developer, so are safe. Even OGNL expressions inserted into data cannot work as Struts 2 (as of 2.5.30) blocks double evaluation, the result of an OGNL expression being interpreted as another expression.

Conclusion

Annotation-based form validation fails at every level. For basic use, official documentation does not state the old conversionError interceptor must be disabled. When creating a micro-architecture, it is incompatible with the ModelDriven standard feature. If reverting to raw HTML instead Struts tags, date formatting is incompatible with conversion errors. For custom data types, code and configuration is torturous, precisely what annotations are supposed to ease.

On top of that, it relies on OGNL, a known attack surface. It's clear an alternate design of annotation-based validation is required.