Detect usage of Apache BeanUtils as dangerous #601

[marcelel]

No description provided.

[codecov]

Codecov Report

Merging #629 (5815c88) into master (fa7f7c4) will not change coverage.
The diff coverage is n/a.

@@            Coverage Diff            @@
##             master     #629   +/-   ##
=========================================
  Coverage     84.90%   84.90%           
  Complexity     1873     1873           
=========================================
  Files           154      154           
  Lines          5036     5036           
  Branches       1124     1124           
=========================================
  Hits           4276     4276           
  Misses          339      339           
  Partials        421      421           

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update fa7f7c4...5815c88. Read the comment docs.

[h3xstream]

Did you reproduce the security in the latest version ?

I don't want to integrate the new signature as it require some testing.
I had done some test before and the class property appears to be blocked in the latest version.

I will documented stuff I fund in this PR.

[h3xstream]

References for a security guard

Release notes

In version 1.9.2 now a special BeanIntrospector class was added which allows
suppressing this property. Note that this BeanIntrospector is NOT enabled by
default! Commons BeanUtils is a low-level library, and on this layer it cannot
be decided whether access to a certain property is legal or not. Therefore,
an application has to activate this suppressing BeanIntrospector explicitly.
This can be done with the following lines of code:

BeanUtilsBean bub = new BeanUtilsBean();
bub.getPropertyUtils().addBeanIntrospector(
    SuppressPropertiesBeanIntrospector.SUPPRESS_CLASS);

http://commons.apache.org/proper/commons-beanutils/javadocs/v1.9.4/RELEASE-NOTES.txt

Jira

We had some internal discussions about this topic. Because BeanUtils is a very low-level library and may be used widely I am reluctant to build in a change which ignores the class property by default. This may break existing code.

BeanUtils 1.9 intorduced the possibility to customize bean introspection. It should be possible to write a custom bean introspector which ignores the class property. We can implement such an introspector and ship it with BeanUtils. In order to make it active, it has to be registered explicitly at a BeanUtilsBean instance or the central BeanUtils object.

https://issues.apache.org/jira/browse/BEANUTILS-463

PR for the fix

Expected behavior taken from a test case..

    public void testAllowAccessToClassProperty() throws Exception {
        final BeanUtilsBean bub = new BeanUtilsBean();
        bub.getPropertyUtils().removeBeanIntrospector(SuppressPropertiesBeanIntrospector.SUPPRESS_CLASS);
        final AlphaBean bean = new AlphaBean();
        String result = bub.getProperty(bean, "class");
        assertEquals("Class property should have been accessed", "class org.apache.commons.beanutils2.AlphaBean", result);
    }

apache/commons-beanutils#7

[marcelel]

No, I didn't reproduce the security. I checked detection of usage BeanUtils.copyProperties. What do you suggest then?

[h3xstream]

This should have been reviewed and merged much sooner.