[FD] Apache Struts2 Spring Plugin Uses a Previously Closed Resource

[Daniel Owens via Fulldisclosure <fulldisclosure@xxxxxxxxxxxx>]

Description
The Apache Struts2 Spring Plugin opens a Spring `ApplicationContex` via a 
try-with-resources block and, outside of said block, then continues to use the 
closed resource, resulting in an uncaught `IllegalStateException`.  This 
particular issue prevents proper functionality and indicates poor code quality 
and code review standards and processes, while not being, itself, otherwise 
excessively meaningful for a production environment (as developer mode should 
not be enabled in such an environment), although it would hamper staging, test, 
and developer environments that are configured in accordance with 
Apache-recommended hot-reload configurations for such environments 
(https://struts.apache.org/plugins/spring/#class-reloading).

Impact
Anyone who follows Struts' documented configuration for this plugin -- enabling 
`struts.devMode`, setting a non-blank 
`struts.objectFactory.spring.class.reloading.watchList`, and declaring 
`ClassReloadingXMLWebApplicationContext` as the root context class in web.xml 
-- will trigger this pathway on every application startup.  Since close() call 
`null`s the context's internal bean factory immediately after `refresh()` 
succeeds, `SpringObjectFactory.setApplicationContext()`call to 
`getBeanFactory()` throws an uncaught `IllegalStateException`, aborting the 
Struts-Spring integration startup process before it can ever finish.  Even if 
the exception were caught, `close()` has already stopped the 
`FilesystemAlterationMonitor` watchdog thread, so the live class-reloading 
capability would never fire a single reload.  As such, the hot-reload feature 
cannot and does not work with this Apache-recommended configuration.

Examples
Sample Code
The code below clearly shows the issue when in developer, a watch list, and 
hot-reloading are enabled.  The try-with-resources calls `.close()` when 
exiting, against `reloadingContext`, which is merely a pointer to `appContext`, 
thereby closing `appContext`.  Immediately after exiting the try-with-resources 
and wrapping conditional, `this. setApplicationContext` is called, passing in 
the now closed `appContext`.  Importantly, ` 
ClassReloadingXMLWebApplicationContext`, to which `appContext` was cast during 
the initialisation of the try-with-resources and against which the `finally` 
that this construct generates (try-with-resources effectively being syntactic 
sugar for a `try{...} finally{reloadingContext.close()}`), has a `close()` 
defined that calls to `super.close()` before removing and stopping the listener.

```java
        String watchList = container.getInstance(String.class, 
SpringConstants.STRUTS_OBJECTFACTORY_SPRING_CLASS_RELOADING_WATCH_LIST);
        String acceptClasses = container.getInstance(String.class, 
SpringConstants.STRUTS_OBJECTFACTORY_SPRING_CLASS_RELOADING_ACCEPT_CLASSES);
        String reloadConfig = container.getInstance(String.class, 
SpringConstants.STRUTS_OBJECTFACTORY_SPRING_CLASS_RELOADING_RELOAD_CONFIG);

        if ("true".equals(devMode)
                && StringUtils.isNotBlank(watchList)
                && appContext instanceof 
ClassReloadingXMLWebApplicationContext) {
            //prevent class caching
            useClassCache = false;

            try (ClassReloadingXMLWebApplicationContext reloadingContext = 
(ClassReloadingXMLWebApplicationContext) appContext) {
                reloadingContext.setupReloading(watchList.split(","), 
acceptClasses, servletContext,
                        "true".equals(reloadConfig));
                LOG.info("Class reloading is enabled. Make sure this is not 
used on a production environment!\n{}",
                        watchList);

                setClassLoader(reloadingContext.getReloadingClassLoader());

                // we need to reload the context, so our isntance of the 
factory is picked up
                reloadingContext.refresh();
            }
        }

        this.setApplicationContext(appContext);
```
Code snippet from `struts2-spring-plugin`'s 
plugins/spring/src/main/java/org/apache/struts2/spring/StrutsSpringObjectFactory.java,
 lines 97-120

```java
    public void close() {
        super.close();

        if (filesystemAlterationMonitor != null) {
            filesystemAlterationMonitor.removeListener(this);
            filesystemAlterationMonitor.stop();
        }
    }
```
Snippet from `struts2-spring-plugin`'s 
plugins/spring/src/main/java/org/apache/struts2/spring/ClassReloadingXMLWebApplicationContext.java,
 lines 157-164, showing the close() override invoked by the implicit 
try-with-resources call
_______________________________________________
Sent through the Full Disclosure mailing list
https://nmap.org/mailman/listinfo/fulldisclosure
Web Archives & RSS: https://seclists.org/fulldisclosure/