WW-5428 Allowlist capability should resolve Hibernate proxies when disableProxyObjects is not set #967
Conversation
kusalk
force-pushed
the
WW-5428-allowlist-hibernate
branch
2 times, most recently
from
dd180d3 to
cd4da6f
Compare
|
Is it ready for review? |
|
@lukaszlenart I need to add coverage, will try get it done this weekend |
There was a problem hiding this comment.
No functional changes in here, I upgraded it to JUnit4 and cleaned it up
| @@ -931,6 +981,15 @@ public void packageInclusion_subclass_both() throws Exception { | |||
| private static String formGetterName(String propertyName) { | |||
| return "get" + propertyName.substring(0, 1).toUpperCase() + propertyName.substring(1); | |||
| } | |||
|
|
|||
| @SuppressWarnings("unchecked") | |||
| private static <T> T mockHibernateProxy(T originalObject, Class<T> proxyInterface) { | |||
There was a problem hiding this comment.
Mocking a Hibernate proxy isn't perfect - integration/acceptance tests would offer better protection against regressions but I'd prefer not to complicate the tests further by introducing a Hibernate session factory etc
kusalk
force-pushed
the
WW-5428-allowlist-hibernate
branch
from
65ff17f to
c965812
Compare
| // If `disallowProxyObjectAccess` is not set, allow resolving Hibernate entities to their underlying | ||
| // classes/members. This allows the allowlist capability to continue working and offer some level of | ||
| // protection in applications where the developer has accepted the risk of allowing OGNL access to Hibernate | ||
| // entities. This is preferred to having to disable the allowlist capability entirely. |
There was a problem hiding this comment.
Wouldn't be good to log this info? Maybe even in WARN level if struts.devMode is enabled, wdyt?
There was a problem hiding this comment.
Hmm yeah doesn't hurt to add some logging - will do
|
WW-5428
The OGNL allowlist capability cannot resolve proxy objects and thus will not work in applications where proxy objects such as Spring beans and Hibernate entities are accessed via OGNL.
Allowing access to such objects via OGNL is not recommended anyway as such access can be used to escalate an SSTI vulnerability.
However, given there seem to be many applications that still rely on accessing Hibernate entities directly, it would still offer much greater security in these cases, if the allowlist were enabled with exemptions for Hibernate proxies, rather than disabled altogether.
As such, in this PR, if an application is configured with
struts.disallowProxyObjectAccess=false, the allowlist capability will resolve the underlying class of any Hibernate proxies and enforce the allowlist against this class instead.