Skip to content

Commit af8af8c

Browse files
author
Costin Leau
committed
+ added fine grained privileged blocks to preserve the caller security stack when invoking the callee
1 parent 81eb114 commit af8af8c

File tree

8 files changed

+371
-73
lines changed

8 files changed

+371
-73
lines changed

‎org.springframework.beans/src/main/java/org/springframework/beans/BeanWrapperImpl.java

Lines changed: 37 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@
2424
import java.lang.reflect.Modifier;
2525
import java.security.AccessControlContext;
2626
import java.security.AccessController;
27+
import java.security.PrivilegedAction;
2728
import java.security.PrivilegedActionException;
2829
import java.security.PrivilegedExceptionAction;
2930
import java.util.ArrayList;
@@ -557,8 +558,18 @@ private Object getPropertyValue(PropertyTokenHolder tokens) throws BeansExceptio
557558
}
558559
final Method readMethod = pd.getReadMethod();
559560
try {
560-
if (!Modifier.isPublic(readMethod.getDeclaringClass().getModifiers())) {
561-
readMethod.setAccessible(true);
561+
if (!Modifier.isPublic(readMethod.getDeclaringClass().getModifiers()) && !readMethod.isAccessible()) {
562+
if (System.getSecurityManager() != null) {
563+
AccessController.doPrivileged(new PrivilegedAction<Object>() {
564+
public Object run() {
565+
readMethod.setAccessible(true);
566+
return null;
567+
}
568+
});
569+
}
570+
else {
571+
readMethod.setAccessible(true);
572+
}
562573
}
563574

564575
Object value = null;
@@ -852,8 +863,18 @@ else if (propValue instanceof Map) {
852863
else {
853864
if (isExtractOldValueForEditor() && pd.getReadMethod() != null) {
854865
final Method readMethod = pd.getReadMethod();
855-
if (!Modifier.isPublic(readMethod.getDeclaringClass().getModifiers())) {
856-
readMethod.setAccessible(true);
866+
if (!Modifier.isPublic(readMethod.getDeclaringClass().getModifiers()) && !readMethod.isAccessible()) {
867+
if (System.getSecurityManager()!= null) {
868+
AccessController.doPrivileged(new PrivilegedAction<Object>() {
869+
public Object run() {
870+
readMethod.setAccessible(true);
871+
return null;
872+
}
873+
});
874+
}
875+
else {
876+
readMethod.setAccessible(true);
877+
}
857878
}
858879
try {
859880
if (System.getSecurityManager() != null) {
@@ -882,8 +903,18 @@ public Object run() throws Exception {
882903
pv.getOriginalPropertyValue().conversionNecessary = (valueToApply != originalValue);
883904
}
884905
final Method writeMethod = pd.getWriteMethod();
885-
if (!Modifier.isPublic(writeMethod.getDeclaringClass().getModifiers())) {
886-
writeMethod.setAccessible(true);
906+
if (!Modifier.isPublic(writeMethod.getDeclaringClass().getModifiers()) && !writeMethod.isAccessible()) {
907+
if (System.getSecurityManager()!= null) {
908+
AccessController.doPrivileged(new PrivilegedAction<Object>() {
909+
public Object run() {
910+
writeMethod.setAccessible(true);
911+
return null;
912+
}
913+
});
914+
}
915+
else {
916+
writeMethod.setAccessible(true);
917+
}
887918
}
888919
final Object value = valueToApply;
889920

‎org.springframework.beans/src/main/java/org/springframework/beans/factory/support/AbstractAutowireCapableBeanFactory.java

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1496,8 +1496,15 @@ protected void invokeCustomInitMethod(String beanName, final Object bean, RootBe
14961496
if (logger.isDebugEnabled()) {
14971497
logger.debug("Invoking init method '" + initMethodName + "' on bean with name '" + beanName + "'");
14981498
}
1499-
ReflectionUtils.makeAccessible(initMethod);
1499+
15001500
if (System.getSecurityManager() != null) {
1501+
AccessController.doPrivileged(new PrivilegedExceptionAction<Object>() {
1502+
public Object run() throws Exception {
1503+
ReflectionUtils.makeAccessible(initMethod);
1504+
return null;
1505+
}
1506+
});
1507+
15011508
try {
15021509
AccessController.doPrivileged(new PrivilegedExceptionAction<Object>() {
15031510

@@ -1514,6 +1521,7 @@ public Object run() throws Exception {
15141521
}
15151522
else {
15161523
try {
1524+
ReflectionUtils.makeAccessible(initMethod);
15171525
initMethod.invoke(bean, (Object[]) null);
15181526
}
15191527
catch (InvocationTargetException ex) {

‎org.springframework.beans/src/main/java/org/springframework/beans/factory/support/AbstractBeanFactory.java

Lines changed: 1 addition & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -209,16 +209,7 @@ protected <T> T doGetBean(
209209
final String name, final Class<T> requiredType, final Object[] args, final boolean typeCheckOnly)
210210
throws BeansException {
211211

212-
if (System.getSecurityManager() != null) {
213-
return AccessController.doPrivileged(new PrivilegedAction<T>() {
214-
public T run() {
215-
return doGetBeanRaw(name, requiredType, args, typeCheckOnly);
216-
}
217-
});
218-
}
219-
else {
220212
return doGetBeanRaw(name, requiredType, args, typeCheckOnly);
221-
}
222213
}
223214
/**
224215
* Return an instance, which may be shared or independent, of the specified bean.
@@ -1446,7 +1437,7 @@ protected void registerDisposableBeanIfNecessary(String beanName, Object bean, R
14461437
@Override
14471438
protected AccessControlContext getAccessControlContext() {
14481439
SecurityContextProvider provider = getSecurityContextProvider();
1449-
return (provider != null ? provider.getAccessControlContext(): null);
1440+
return (provider != null ? provider.getAccessControlContext(): AccessController.getContext());
14501441
}
14511442

14521443
/**

‎org.springframework.beans/src/main/java/org/springframework/beans/factory/support/ConstructorResolver.java

Lines changed: 17 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -385,8 +385,23 @@ public BeanWrapper instantiateUsingFactoryMethod(final String beanName, final Ro
385385
// Need to determine the factory method...
386386
// Try all methods with this name to see if they match the given arguments.
387387
factoryClass = ClassUtils.getUserClass(factoryClass);
388-
Method[] rawCandidates = (mbd.isNonPublicAccessAllowed() ?
389-
ReflectionUtils.getAllDeclaredMethods(factoryClass) : factoryClass.getMethods());
388+
Method[] rawCandidates = null;
389+
390+
final Class factoryClazz = factoryClass;
391+
if (System.getSecurityManager() != null) {
392+
393+
rawCandidates = AccessController.doPrivileged(new PrivilegedAction<Method[]>() {
394+
public Method[] run() {
395+
return (mbd.isNonPublicAccessAllowed() ?
396+
ReflectionUtils.getAllDeclaredMethods(factoryClazz) : factoryClazz.getMethods());
397+
}
398+
});
399+
}
400+
else {
401+
rawCandidates = (mbd.isNonPublicAccessAllowed() ?
402+
ReflectionUtils.getAllDeclaredMethods(factoryClazz) : factoryClazz.getMethods());
403+
}
404+
390405
List<Method> candidateSet = new ArrayList<Method>();
391406
for (Method candidate : rawCandidates) {
392407
if (Modifier.isStatic(candidate.getModifiers()) == isStatic &&

‎org.springframework.beans/src/main/java/org/springframework/beans/factory/support/DisposableBeanAdapter.java

Lines changed: 26 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121
import java.lang.reflect.Method;
2222
import java.security.AccessControlContext;
2323
import java.security.AccessController;
24+
import java.security.PrivilegedAction;
2425
import java.security.PrivilegedActionException;
2526
import java.security.PrivilegedExceptionAction;
2627
import java.util.ArrayList;
@@ -81,7 +82,7 @@ class DisposableBeanAdapter implements DisposableBean, Runnable, Serializable {
8182
* @param postProcessors the List of BeanPostProcessors
8283
* (potentially DestructionAwareBeanPostProcessor), if any
8384
*/
84-
public DisposableBeanAdapter(Object bean, String beanName, RootBeanDefinition beanDefinition,
85+
public DisposableBeanAdapter(final Object bean, String beanName, RootBeanDefinition beanDefinition,
8586
List<BeanPostProcessor> postProcessors, AccessControlContext acc) {
8687

8788
Assert.notNull(bean, "Bean must not be null");
@@ -92,14 +93,26 @@ public DisposableBeanAdapter(Object bean, String beanName, RootBeanDefinition be
9293
this.nonPublicAccessAllowed = beanDefinition.isNonPublicAccessAllowed();
9394
this.acc = acc;
9495

95-
String destroyMethodName = beanDefinition.getDestroyMethodName();
96+
final String destroyMethodName = beanDefinition.getDestroyMethodName();
9697
if (destroyMethodName != null && !(this.invokeDisposableBean && "destroy".equals(destroyMethodName)) &&
9798
!beanDefinition.isExternallyManagedDestroyMethod(destroyMethodName)) {
9899
this.destroyMethodName = destroyMethodName;
99100
try {
100-
this.destroyMethod = (this.nonPublicAccessAllowed ?
101-
BeanUtils.findMethodWithMinimalParameters(bean.getClass(), destroyMethodName) :
102-
BeanUtils.findMethodWithMinimalParameters(bean.getClass().getMethods(), destroyMethodName));
101+
if (System.getSecurityManager() != null) {
102+
AccessController.doPrivileged(new PrivilegedAction<Object>() {
103+
public Object run() {
104+
destroyMethod = (nonPublicAccessAllowed ?
105+
BeanUtils.findMethodWithMinimalParameters(bean.getClass(), destroyMethodName) :
106+
BeanUtils.findMethodWithMinimalParameters(bean.getClass().getMethods(), destroyMethodName));
107+
return null;
108+
}
109+
});
110+
}
111+
else {
112+
this.destroyMethod = (this.nonPublicAccessAllowed ?
113+
BeanUtils.findMethodWithMinimalParameters(bean.getClass(), destroyMethodName) :
114+
BeanUtils.findMethodWithMinimalParameters(bean.getClass().getMethods(), destroyMethodName));
115+
}
103116
}
104117
catch (IllegalArgumentException ex) {
105118
throw new BeanDefinitionValidationException("Couldn't find a unique destroy method on bean with name '" +
@@ -229,9 +242,15 @@ private void invokeCustomDestroyMethod(final Method destroyMethod) {
229242
logger.debug("Invoking destroy method '" + this.destroyMethodName +
230243
"' on bean with name '" + this.beanName + "'");
231244
}
232-
ReflectionUtils.makeAccessible(destroyMethod);
233245
try {
234246
if (System.getSecurityManager() != null) {
247+
AccessController.doPrivileged(new PrivilegedAction<Object>() {
248+
public Object run() {
249+
ReflectionUtils.makeAccessible(destroyMethod);
250+
return null;
251+
}
252+
});
253+
235254
try {
236255
AccessController.doPrivileged(new PrivilegedExceptionAction<Object>() {
237256
public Object run() throws Exception {
@@ -244,6 +263,7 @@ public Object run() throws Exception {
244263
}
245264
}
246265
else {
266+
ReflectionUtils.makeAccessible(destroyMethod);
247267
destroyMethod.invoke(bean, args);
248268
}
249269
} catch (InvocationTargetException ex) {

‎org.springframework.beans/src/main/java/org/springframework/beans/factory/support/SimpleInstantiationStrategy.java

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -89,9 +89,18 @@ protected Object instantiateWithMethodInjection(
8989

9090
public Object instantiate(
9191
RootBeanDefinition beanDefinition, String beanName, BeanFactory owner,
92-
Constructor ctor, Object[] args) {
92+
final Constructor ctor, Object[] args) {
9393

9494
if (beanDefinition.getMethodOverrides().isEmpty()) {
95+
if (System.getSecurityManager() != null) {
96+
// use own privileged to change accessibility (when security is on)
97+
AccessController.doPrivileged(new PrivilegedAction<Object>() {
98+
public Object run() {
99+
ReflectionUtils.makeAccessible(ctor);
100+
return null;
101+
}
102+
});
103+
}
95104
return BeanUtils.instantiateClass(ctor, args);
96105
}
97106
else {

0 commit comments

Comments
 (0)