struts-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Musachy Barroso" <>
Subject hacking OGNL and parameter binding
Date Thu, 17 Jul 2008 21:28:52 GMT
Yeah I am set to fix those security holes ;). Doing the change below,
all tests pass, with the exception of some tests in
ParameterInterceptorTest, that need to inject dependencies, and others
that check for the order of the values added to the stack (new context
is created here, so they fail)

+        ValueStack emptyStack = valueStackFactory.createValueStack(stack);
+        Map<String, Object> context = emptyStack.getContext();
+        ((OgnlContext)context).getValues().clear(); /// THIS IS BAD
+        ReflectionContextState.setCreatingNullObjects(context, true);
+        ReflectionContextState.setDenyMethodExecution(context, true);
+        ReflectionContextState.setReportingConversionErrors(context, true);
         for (Map.Entry<String, Object> entry :
acceptableParameters.entrySet()) {
             String name = entry.getKey();
             Object value = entry.getValue();
@@ -233,7 +265,7 @@
             String name = entry.getKey();
             Object value = entry.getValue();
             try {
-                stack.setValue(name, value);
+                emptyStack.setValue(name, value);
             } catch (RuntimeException e) {
                 if (devMode) {
                     String developerNotification =
"devmode.notification", ActionContext.getContext().getLocale(),
"Developer Notification:\n{0}", new Object[]{
@@ -246,6 +278,9 @@
+        stack.getContext().putAll(acceptableParameters);

The 2 big things to be addressed are:

1. ((OgnlContext)context).getValues().clear();

I cannot just do context.clear(), because that method not only removes
the values from the stack, but it clears the root, type converter and
other stuff, so we will have to add another "clear" method to the
OgnlContext, that just clears the values.

2. throwPropertyExceptions which needs to be the same in the new value
stack, but I think it is getting cleared.

what do you guys think?

"Hey you! Would you help me to carry the stone?" Pink Floyd

To unsubscribe, e-mail:
For additional commands, e-mail:

View raw message