Skip to content

Commit 6be761a

Browse files
cesarhernandezgtcesarhernandezgt
authored
Merge pull request spring-projects#2 from cesarhernandezgt/v.4.3.30.RELEASE-TT.x-patch
backport for CVE-2022-22970 and CVE-2022-22971
2 parents 4d6dc0c + 023aea3 commit 6be761a

File tree

6 files changed

+208
-73
lines changed

6 files changed

+208
-73
lines changed

‎spring-beans/src/main/java/org/springframework/beans/CachedIntrospectionResults.java

Lines changed: 78 additions & 58 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2002-2020 the original author or authors.
2+
* Copyright 2002-2022 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -20,6 +20,7 @@
2020
import java.beans.IntrospectionException;
2121
import java.beans.Introspector;
2222
import java.beans.PropertyDescriptor;
23+
import java.net.URL;
2324
import java.security.ProtectionDomain;
2425
import java.util.Collections;
2526
import java.util.Iterator;
@@ -30,6 +31,7 @@
3031
import java.util.concurrent.ConcurrentHashMap;
3132
import java.util.concurrent.ConcurrentMap;
3233

34+
3335
import org.apache.commons.logging.Log;
3436
import org.apache.commons.logging.LogFactory;
3537

@@ -93,11 +95,13 @@ public class CachedIntrospectionResults {
9395
*/
9496
public static final String IGNORE_BEANINFO_PROPERTY_NAME = "spring.beaninfo.ignore";
9597

98+
private static final PropertyDescriptor[] EMPTY_PROPERTY_DESCRIPTOR_ARRAY = {};
99+
96100

97101
private static final boolean shouldIntrospectorIgnoreBeaninfoClasses =
98102
SpringProperties.getFlag(IGNORE_BEANINFO_PROPERTY_NAME);
99103

100-
/** Stores the BeanInfoFactory instances */
104+
/** Stores the BeanInfoFactory instances. */
101105
private static final List<BeanInfoFactory> beanInfoFactories = SpringFactoriesLoader.loadFactories(
102106
BeanInfoFactory.class, CachedIntrospectionResults.class.getClassLoader());
103107

@@ -176,7 +180,6 @@ public static void clearClassLoader(ClassLoader classLoader) {
176180
* @return the corresponding CachedIntrospectionResults
177181
* @throws BeansException in case of introspection failure
178182
*/
179-
@SuppressWarnings("unchecked")
180183
static CachedIntrospectionResults forClass(Class<?> beanClass) throws BeansException {
181184
CachedIntrospectionResults results = strongClassCache.get(beanClass);
182185
if (results != null) {
@@ -244,14 +247,32 @@ private static boolean isUnderneathClassLoader(ClassLoader candidate, ClassLoade
244247
return false;
245248
}
246249

250+
/**
251+
* Retrieve a {@link BeanInfo} descriptor for the given target class.
252+
* @param beanClass the target class to introspect
253+
* @return the resulting {@code BeanInfo} descriptor (never {@code null})
254+
* @throws IntrospectionException from the underlying {@link Introspector}
255+
*/
256+
private static BeanInfo getBeanInfo(Class<?> beanClass) throws IntrospectionException {
257+
for (BeanInfoFactory beanInfoFactory : beanInfoFactories) {
258+
BeanInfo beanInfo = beanInfoFactory.getBeanInfo(beanClass);
259+
if (beanInfo != null) {
260+
return beanInfo;
261+
}
262+
}
263+
return (shouldIntrospectorIgnoreBeaninfoClasses ?
264+
Introspector.getBeanInfo(beanClass, Introspector.IGNORE_ALL_BEANINFO) :
265+
Introspector.getBeanInfo(beanClass));
266+
}
267+
247268

248-
/** The BeanInfo object for the introspected bean class */
269+
/** The BeanInfo object for the introspected bean class. */
249270
private final BeanInfo beanInfo;
250271

251-
/** PropertyDescriptor objects keyed by property name String */
252-
private final Map<String, PropertyDescriptor> propertyDescriptorCache;
272+
/** PropertyDescriptor objects keyed by property name String. */
273+
private final Map<String, PropertyDescriptor> propertyDescriptors;
253274

254-
/** TypeDescriptor objects keyed by PropertyDescriptor */
275+
/** TypeDescriptor objects keyed by PropertyDescriptor. */
255276
private final ConcurrentMap<PropertyDescriptor, TypeDescriptor> typeDescriptorCache;
256277

257278

@@ -265,37 +286,27 @@ private CachedIntrospectionResults(Class<?> beanClass) throws BeansException {
265286
if (logger.isTraceEnabled()) {
266287
logger.trace("Getting BeanInfo for class [" + beanClass.getName() + "]");
267288
}
268-
269-
BeanInfo beanInfo = null;
270-
for (BeanInfoFactory beanInfoFactory : beanInfoFactories) {
271-
beanInfo = beanInfoFactory.getBeanInfo(beanClass);
272-
if (beanInfo != null) {
273-
break;
274-
}
275-
}
276-
if (beanInfo == null) {
277-
// If none of the factories supported the class, fall back to the default
278-
beanInfo = (shouldIntrospectorIgnoreBeaninfoClasses ?
279-
Introspector.getBeanInfo(beanClass, Introspector.IGNORE_ALL_BEANINFO) :
280-
Introspector.getBeanInfo(beanClass));
281-
}
282-
this.beanInfo = beanInfo;
289+
this.beanInfo = getBeanInfo(beanClass);
283290

284291
if (logger.isTraceEnabled()) {
285292
logger.trace("Caching PropertyDescriptors for class [" + beanClass.getName() + "]");
286293
}
287-
this.propertyDescriptorCache = new LinkedHashMap<String, PropertyDescriptor>();
294+
this.propertyDescriptors = new LinkedHashMap<String, PropertyDescriptor>();
288295

289296
// This call is slow so we do it once.
290297
PropertyDescriptor[] pds = this.beanInfo.getPropertyDescriptors();
291298
for (PropertyDescriptor pd : pds) {
292-
if (Class.class == beanClass && (!"name".equals(pd.getName()) && !pd.getName().endsWith("Name"))) {
299+
if (Class.class == beanClass && !("name".equals(pd.getName()) ||
300+
(pd.getName().endsWith("Name") && String.class == pd.getPropertyType()))) {
293301
// Only allow all name variants of Class properties
294302
continue;
295303
}
296-
if (pd.getPropertyType() != null && (ClassLoader.class.isAssignableFrom(pd.getPropertyType())
297-
|| ProtectionDomain.class.isAssignableFrom(pd.getPropertyType()))) {
298-
// Ignore ClassLoader and ProtectionDomain types - nobody needs to bind to those
304+
if (URL.class == beanClass && "content".equals(pd.getName())) {
305+
// Only allow URL attribute introspection, not content resolution
306+
continue;
307+
}
308+
if (pd.getWriteMethod() == null && isInvalidReadOnlyPropertyType(pd.getPropertyType())) {
309+
// Ignore read-only properties such as ClassLoader - no need to bind to those
299310
continue;
300311
}
301312
if (logger.isTraceEnabled()) {
@@ -305,30 +316,15 @@ private CachedIntrospectionResults(Class<?> beanClass) throws BeansException {
305316
"; editor [" + pd.getPropertyEditorClass().getName() + "]" : ""));
306317
}
307318
pd = buildGenericTypeAwarePropertyDescriptor(beanClass, pd);
308-
this.propertyDescriptorCache.put(pd.getName(), pd);
319+
this.propertyDescriptors.put(pd.getName(), pd);
309320
}
310321

311322
// Explicitly check implemented interfaces for setter/getter methods as well,
312323
// in particular for Java 8 default methods...
313-
Class<?> clazz = beanClass;
314-
while (clazz != null) {
315-
Class<?>[] ifcs = clazz.getInterfaces();
316-
for (Class<?> ifc : ifcs) {
317-
BeanInfo ifcInfo = Introspector.getBeanInfo(ifc, Introspector.IGNORE_ALL_BEANINFO);
318-
PropertyDescriptor[] ifcPds = ifcInfo.getPropertyDescriptors();
319-
for (PropertyDescriptor pd : ifcPds) {
320-
if (!this.propertyDescriptorCache.containsKey(pd.getName())) {
321-
pd = buildGenericTypeAwarePropertyDescriptor(beanClass, pd);
322-
if (pd.getPropertyType() != null && (ClassLoader.class.isAssignableFrom(pd.getPropertyType())
323-
|| ProtectionDomain.class.isAssignableFrom(pd.getPropertyType()))) {
324-
// Ignore ClassLoader and ProtectionDomain types - nobody needs to bind to those
325-
continue;
326-
}
327-
this.propertyDescriptorCache.put(pd.getName(), pd);
328-
}
329-
}
330-
}
331-
clazz = clazz.getSuperclass();
324+
Class<?> currClass = beanClass;
325+
while (currClass != null && currClass != Object.class) {
326+
introspectInterfaces(beanClass, currClass);
327+
currClass = currClass.getSuperclass();
332328
}
333329

334330
this.typeDescriptorCache = new ConcurrentReferenceHashMap<PropertyDescriptor, TypeDescriptor>();
@@ -338,6 +334,35 @@ private CachedIntrospectionResults(Class<?> beanClass) throws BeansException {
338334
}
339335
}
340336

337+
private void introspectInterfaces(Class<?> beanClass, Class<?> currClass) throws IntrospectionException {
338+
for (Class<?> ifc : currClass.getInterfaces()) {
339+
if (!ClassUtils.isJavaLanguageInterface(ifc)) {
340+
for (PropertyDescriptor pd : getBeanInfo(ifc).getPropertyDescriptors()) {
341+
PropertyDescriptor existingPd = this.propertyDescriptors.get(pd.getName());
342+
if (existingPd == null ||
343+
(existingPd.getReadMethod() == null && pd.getReadMethod() != null)) {
344+
// GenericTypeAwarePropertyDescriptor leniently resolves a set* write method
345+
// against a declared read method, so we prefer read method descriptors here.
346+
pd = buildGenericTypeAwarePropertyDescriptor(beanClass, pd);
347+
if (pd.getWriteMethod() == null && isInvalidReadOnlyPropertyType(pd.getPropertyType())) {
348+
// Ignore read-only properties such as ClassLoader - no need to bind to those
349+
continue;
350+
}
351+
this.propertyDescriptors.put(pd.getName(), pd);
352+
}
353+
}
354+
introspectInterfaces(ifc, ifc);
355+
}
356+
}
357+
}
358+
359+
private boolean isInvalidReadOnlyPropertyType(Class<?> returnType) {
360+
return (returnType != null && (AutoCloseable.class.isAssignableFrom(returnType) ||
361+
ClassLoader.class.isAssignableFrom(returnType) ||
362+
ProtectionDomain.class.isAssignableFrom(returnType)));
363+
}
364+
365+
341366
BeanInfo getBeanInfo() {
342367
return this.beanInfo;
343368
}
@@ -346,28 +371,22 @@ Class<?> getBeanClass() {
346371
return this.beanInfo.getBeanDescriptor().getBeanClass();
347372
}
348373

374+
349375
PropertyDescriptor getPropertyDescriptor(String name) {
350-
PropertyDescriptor pd = this.propertyDescriptorCache.get(name);
376+
PropertyDescriptor pd = this.propertyDescriptors.get(name);
351377
if (pd == null && StringUtils.hasLength(name)) {
352378
// Same lenient fallback checking as in Property...
353-
pd = this.propertyDescriptorCache.get(StringUtils.uncapitalize(name));
379+
pd = this.propertyDescriptors.get(StringUtils.uncapitalize(name));
354380
if (pd == null) {
355-
pd = this.propertyDescriptorCache.get(StringUtils.capitalize(name));
381+
pd = this.propertyDescriptors.get(StringUtils.capitalize(name));
356382
}
357383
}
358384
return (pd == null || pd instanceof GenericTypeAwarePropertyDescriptor ? pd :
359385
buildGenericTypeAwarePropertyDescriptor(getBeanClass(), pd));
360386
}
361387

362388
PropertyDescriptor[] getPropertyDescriptors() {
363-
PropertyDescriptor[] pds = new PropertyDescriptor[this.propertyDescriptorCache.size()];
364-
int i = 0;
365-
for (PropertyDescriptor pd : this.propertyDescriptorCache.values()) {
366-
pds[i] = (pd instanceof GenericTypeAwarePropertyDescriptor ? pd :
367-
buildGenericTypeAwarePropertyDescriptor(getBeanClass(), pd));
368-
i++;
369-
}
370-
return pds;
389+
return this.propertyDescriptors.values().toArray(EMPTY_PROPERTY_DESCRIPTOR_ARRAY);
371390
}
372391

373392
private PropertyDescriptor buildGenericTypeAwarePropertyDescriptor(Class<?> beanClass, PropertyDescriptor pd) {
@@ -385,6 +404,7 @@ TypeDescriptor addTypeDescriptor(PropertyDescriptor pd, TypeDescriptor td) {
385404
return (existing != null ? existing : td);
386405
}
387406

407+
388408
TypeDescriptor getTypeDescriptor(PropertyDescriptor pd) {
389409
return this.typeDescriptorCache.get(pd);
390410
}

‎spring-beans/src/test/java/org/springframework/beans/BeanWrapperTests.java

Lines changed: 46 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,15 +16,20 @@
1616

1717
package org.springframework.beans;
1818

19+
import java.net.MalformedURLException;
1920
import java.util.Collections;
2021
import java.util.Map;
2122
import java.util.Optional;
2223

2324
import org.junit.Test;
2425

26+
import org.springframework.core.OverridingClassLoader;
27+
import org.springframework.core.io.DefaultResourceLoader;
28+
import org.springframework.core.io.UrlResource;
2529
import org.springframework.tests.sample.beans.TestBean;
2630

2731
import static org.junit.Assert.*;
32+
import static org.junit.Assert.assertEquals;
2833

2934
/**
3035
* Specific {@link BeanWrapperImpl} tests.
@@ -153,7 +158,7 @@ public void setPropertyTypeMismatch() {
153158
}
154159

155160
@Test
156-
public void propertyDescriptors() {
161+
public void propertyDescriptors() throws MalformedURLException {
157162
TestBean target = new TestBean();
158163
target.setSpouse(new TestBean());
159164
BeanWrapper accessor = createAccessor(target);
@@ -165,6 +170,46 @@ public void propertyDescriptors() {
165170
assertEquals("b", accessor.getPropertyValue("spouse.name"));
166171
assertEquals(String.class, accessor.getPropertyDescriptor("name").getPropertyType());
167172
assertEquals(String.class, accessor.getPropertyDescriptor("spouse.name").getPropertyType());
173+
174+
assertFalse(accessor.isReadableProperty("class.package"));
175+
assertFalse(accessor.isReadableProperty("class.module"));
176+
assertFalse(accessor.isReadableProperty("class.classLoader"));
177+
assertTrue(accessor.isReadableProperty("class.name"));
178+
assertTrue(accessor.isReadableProperty("class.simpleName"));
179+
assertEquals(TestBean.class.getName(), accessor.getPropertyValue("class.name"));
180+
assertEquals(TestBean.class.getSimpleName(), accessor.getPropertyValue("class.simpleName"));
181+
assertEquals(String.class, accessor.getPropertyDescriptor("class.name").getPropertyType());
182+
assertEquals(String.class, accessor.getPropertyDescriptor("class.simpleName").getPropertyType());
183+
184+
accessor = createAccessor(new DefaultResourceLoader());
185+
186+
assertFalse(accessor.isReadableProperty("class.package"));
187+
assertFalse(accessor.isReadableProperty("class.module"));
188+
assertFalse(accessor.isReadableProperty("class.classLoader"));
189+
assertTrue(accessor.isReadableProperty("class.name"));
190+
assertTrue(accessor.isReadableProperty("class.simpleName"));
191+
assertTrue(accessor.isReadableProperty("classLoader"));
192+
assertTrue(accessor.isWritableProperty("classLoader"));
193+
OverridingClassLoader ocl = new OverridingClassLoader(getClass().getClassLoader());
194+
accessor.setPropertyValue("classLoader", ocl);
195+
196+
assertEquals(ocl,accessor.getPropertyValue("classLoader"));
197+
198+
accessor = createAccessor(new UrlResource("https://spring.io"));
199+
200+
assertFalse(accessor.isReadableProperty("class.package"));
201+
assertFalse(accessor.isReadableProperty("class.module"));
202+
assertFalse(accessor.isReadableProperty("class.classLoader"));
203+
assertTrue(accessor.isReadableProperty("class.name"));
204+
assertTrue(accessor.isReadableProperty("class.simpleName"));
205+
assertTrue(accessor.isReadableProperty("URL.protocol"));
206+
assertTrue(accessor.isReadableProperty("URL.host"));
207+
assertTrue(accessor.isReadableProperty("URL.port"));
208+
assertTrue(accessor.isReadableProperty("URL.file"));
209+
assertFalse(accessor.isReadableProperty("URL.content"));
210+
assertFalse(accessor.isReadableProperty("inputStream"));
211+
assertTrue(accessor.isReadableProperty("filename"));
212+
assertTrue(accessor.isReadableProperty("description"));
168213
}
169214

170215
@Test

‎spring-core/src/main/java/org/springframework/util/ClassUtils.java

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,9 @@
1717
package org.springframework.util;
1818

1919
import java.beans.Introspector;
20+
import java.io.Closeable;
21+
import java.io.Externalizable;
22+
import java.io.Serializable;
2023
import java.lang.reflect.Array;
2124
import java.lang.reflect.Constructor;
2225
import java.lang.reflect.Method;
@@ -96,6 +99,11 @@ public abstract class ClassUtils {
9699
*/
97100
private static final Map<String, Class<?>> commonClassCache = new HashMap<String, Class<?>>(32);
98101

102+
/**
103+
* Common Java language interfaces which are supposed to be ignored
104+
* when searching for 'primary' user-level interfaces.
105+
*/
106+
private static final Set<Class<?>> javaLanguageInterfaces;
99107

100108
static {
101109
primitiveWrapperTypeMap.put(Boolean.class, boolean.class);
@@ -129,6 +137,10 @@ public abstract class ClassUtils {
129137
registerCommonClasses(Throwable.class, Exception.class, RuntimeException.class,
130138
Error.class, StackTraceElement.class, StackTraceElement[].class);
131139
registerCommonClasses(Enum.class, Iterable.class, Cloneable.class, Comparable.class);
140+
Class<?>[] javaLanguageInterfaceArray = {Serializable.class, Externalizable.class,
141+
Closeable.class, AutoCloseable.class, Cloneable.class, Comparable.class};
142+
registerCommonClasses(javaLanguageInterfaceArray);
143+
javaLanguageInterfaces = new HashSet<Class<?>>(Arrays.asList(javaLanguageInterfaceArray));
132144
}
133145

134146

@@ -746,6 +758,19 @@ public static Class<?> determineCommonAncestor(Class<?> clazz1, Class<?> clazz2)
746758
return ancestor;
747759
}
748760

761+
/**
762+
* Determine whether the given interface is a common Java language interface:
763+
* {@link Serializable}, {@link Externalizable}, {@link Closeable}, {@link AutoCloseable},
764+
* {@link Cloneable}, {@link Comparable} - all of which can be ignored when looking
765+
* for 'primary' user-level interfaces. Common characteristics: no service-level
766+
* operations, no bean property methods, no default methods.
767+
* @param ifc the interface to check
768+
* @since 5.0.3
769+
*/
770+
public static boolean isJavaLanguageInterface(Class<?> ifc) {
771+
return javaLanguageInterfaces.contains(ifc);
772+
}
773+
749774
/**
750775
* Check whether the given object is a CGLIB proxy.
751776
* @param object the object to check

0 commit comments

Comments
 (0)