[jboss-cvs] JBossAS SVN: r71122 - in projects/jboss-reflect/trunk/src: tests/org/jboss/test/beaninfo/support and 2 other directories.
jboss-cvs-commits at lists.jboss.org
jboss-cvs-commits at lists.jboss.org
Fri Mar 21 00:37:45 EDT 2008
Author: adrian at jboss.org
Date: 2008-03-21 00:37:44 -0400 (Fri, 21 Mar 2008)
New Revision: 71122
Added:
projects/jboss-reflect/trunk/src/tests/org/jboss/test/beaninfo/support/FieldsClass.java
projects/jboss-reflect/trunk/src/tests/org/jboss/test/beaninfo/test/AccessRestrictionTestCase.java
Modified:
projects/jboss-reflect/trunk/src/main/org/jboss/reflect/plugins/introspection/IntrospectionTypeInfoFactoryImpl.java
projects/jboss-reflect/trunk/src/main/org/jboss/reflect/plugins/introspection/ReflectFieldInfoImpl.java
projects/jboss-reflect/trunk/src/tests/org/jboss/test/beaninfo/test/BeanInfoTestSuite.java
projects/jboss-reflect/trunk/src/tests/org/jboss/test/classinfo/test/AccessRestrictionTestCase.java
Log:
[JBREFLECT-22] - Plug the security holes in the non-public field access
Modified: projects/jboss-reflect/trunk/src/main/org/jboss/reflect/plugins/introspection/IntrospectionTypeInfoFactoryImpl.java
===================================================================
--- projects/jboss-reflect/trunk/src/main/org/jboss/reflect/plugins/introspection/IntrospectionTypeInfoFactoryImpl.java 2008-03-21 04:18:26 UTC (rev 71121)
+++ projects/jboss-reflect/trunk/src/main/org/jboss/reflect/plugins/introspection/IntrospectionTypeInfoFactoryImpl.java 2008-03-21 04:37:44 UTC (rev 71122)
@@ -168,22 +168,28 @@
}
@SuppressWarnings("deprecation")
- public FieldInfoImpl[] getFields(ClassInfoImpl classInfo)
+ public FieldInfoImpl[] getFields(final ClassInfoImpl classInfo)
{
- Class<?> clazz = classInfo.getType();
- Field[] fields = getDeclaredFields(clazz);
- if (fields == null || fields.length == 0)
- return null;
-
- ReflectFieldInfoImpl[] infos = new ReflectFieldInfoImpl[fields.length];
- for (int i = 0; i < fields.length; ++i)
+ return AccessController.doPrivileged(new PrivilegedAction<FieldInfoImpl[]>()
{
- AnnotationValue[] annotations = getAnnotations(fields[i]);
- infos[i] = new ReflectFieldInfoImpl(annotations, fields[i].getName(), getTypeInfo(fields[i].getGenericType()), fields[i].getModifiers(), (ClassInfo) getTypeInfo(fields[i].getDeclaringClass()));
- infos[i].setField(fields[i]);
- }
+ public FieldInfoImpl[] run()
+ {
+ Class<?> clazz = classInfo.getType();
+ Field[] fields = getDeclaredFields(clazz);
+ if (fields == null || fields.length == 0)
+ return null;
- return infos;
+ ReflectFieldInfoImpl[] infos = new ReflectFieldInfoImpl[fields.length];
+ for (int i = 0; i < fields.length; ++i)
+ {
+ AnnotationValue[] annotations = getAnnotations(fields[i]);
+ infos[i] = new ReflectFieldInfoImpl(annotations, fields[i].getName(), getTypeInfo(fields[i].getGenericType()), fields[i].getModifiers(), (ClassInfo) getTypeInfo(fields[i].getDeclaringClass()));
+ infos[i].setField(fields[i]);
+ }
+
+ return infos;
+ }
+ });
}
@SuppressWarnings("deprecation")
Modified: projects/jboss-reflect/trunk/src/main/org/jboss/reflect/plugins/introspection/ReflectFieldInfoImpl.java
===================================================================
--- projects/jboss-reflect/trunk/src/main/org/jboss/reflect/plugins/introspection/ReflectFieldInfoImpl.java 2008-03-21 04:18:26 UTC (rev 71121)
+++ projects/jboss-reflect/trunk/src/main/org/jboss/reflect/plugins/introspection/ReflectFieldInfoImpl.java 2008-03-21 04:37:44 UTC (rev 71122)
@@ -24,6 +24,7 @@
import java.io.IOException;
import java.io.ObjectInputStream;
import java.lang.reflect.Field;
+import java.lang.reflect.Modifier;
import java.lang.reflect.ReflectPermission;
import java.security.AccessController;
import java.security.Permission;
@@ -80,6 +81,7 @@
*/
public void setField(Field field)
{
+ accessCheck(Modifier.isPublic(field.getModifiers()));
this.field = field;
if (isPublic() == false && field != null)
setAccessible();
@@ -92,15 +94,26 @@
*/
public Field getField()
{
+ accessCheck();
return field;
}
/**
* Check access permission.
*/
- protected void accessCheck()
+ protected final void accessCheck() // final because we don't want subclasses to disable it
{
- if (isPublic() == false)
+ accessCheck(isPublic());
+ }
+
+ /**
+ * Check access permission.
+ *
+ * @param isPublic whether the field is public
+ */
+ protected final void accessCheck(final boolean isPublic) // final because we don't want subclasses to disable it
+ {
+ if (isPublic == false)
{
SecurityManager sm = System.getSecurityManager();
if (sm != null)
Added: projects/jboss-reflect/trunk/src/tests/org/jboss/test/beaninfo/support/FieldsClass.java
===================================================================
--- projects/jboss-reflect/trunk/src/tests/org/jboss/test/beaninfo/support/FieldsClass.java (rev 0)
+++ projects/jboss-reflect/trunk/src/tests/org/jboss/test/beaninfo/support/FieldsClass.java 2008-03-21 04:37:44 UTC (rev 71122)
@@ -0,0 +1,43 @@
+/*
+* JBoss, Home of Professional Open Source
+* Copyright 2006, JBoss Inc., and individual contributors as indicated
+* by the @authors tag. See the copyright.txt in the distribution for a
+* full listing of individual contributors.
+*
+* This is free software; you can redistribute it and/or modify it
+* under the terms of the GNU Lesser General Public License as
+* published by the Free Software Foundation; either version 2.1 of
+* the License, or (at your option) any later version.
+*
+* This software is distributed in the hope that it will be useful,
+* but WITHOUT ANY WARRANTY; without even the implied warranty of
+* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+* Lesser General Public License for more details.
+*
+* You should have received a copy of the GNU Lesser General Public
+* License along with this software; if not, write to the Free
+* Software Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA
+* 02110-1301 USA, or see the FSF site: http://www.fsf.org.
+*/
+package org.jboss.test.beaninfo.support;
+
+/**
+ * @author <a href="mailto:ales.justin at jboss.com">Ales Justin</a>
+ */
+public class FieldsClass
+{
+ @SuppressWarnings("unused")
+ private String privString;
+ protected String protString;
+ public String pubString;
+
+ public String getPrivStringNotGetter()
+ {
+ return privString;
+ }
+
+ public String getProtStringNotGetter()
+ {
+ return protString;
+ }
+}
Added: projects/jboss-reflect/trunk/src/tests/org/jboss/test/beaninfo/test/AccessRestrictionTestCase.java
===================================================================
--- projects/jboss-reflect/trunk/src/tests/org/jboss/test/beaninfo/test/AccessRestrictionTestCase.java (rev 0)
+++ projects/jboss-reflect/trunk/src/tests/org/jboss/test/beaninfo/test/AccessRestrictionTestCase.java 2008-03-21 04:37:44 UTC (rev 71122)
@@ -0,0 +1,153 @@
+/*
+* JBoss, Home of Professional Open Source
+* Copyright 2006, JBoss Inc., and individual contributors as indicated
+* by the @authors tag. See the copyright.txt in the distribution for a
+* full listing of individual contributors.
+*
+* This is free software; you can redistribute it and/or modify it
+* under the terms of the GNU Lesser General Public License as
+* published by the Free Software Foundation; either version 2.1 of
+* the License, or (at your option) any later version.
+*
+* This software is distributed in the hope that it will be useful,
+* but WITHOUT ANY WARRANTY; without even the implied warranty of
+* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+* Lesser General Public License for more details.
+*
+* You should have received a copy of the GNU Lesser General Public
+* License along with this software; if not, write to the Free
+* Software Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA
+* 02110-1301 USA, or see the FSF site: http://www.fsf.org.
+*/
+package org.jboss.test.beaninfo.test;
+
+import java.security.AccessControlException;
+
+import junit.framework.Test;
+
+import org.jboss.beans.info.spi.BeanAccessMode;
+import org.jboss.beans.info.spi.BeanInfo;
+import org.jboss.beans.info.spi.PropertyInfo;
+import org.jboss.config.plugins.BasicConfiguration;
+import org.jboss.config.spi.Configuration;
+import org.jboss.test.AbstractTestCaseWithSetup;
+import org.jboss.test.AbstractTestDelegate;
+import org.jboss.test.beaninfo.support.FieldsClass;
+
+/**
+ * Access restriction test.
+ *
+ * @author <a href="ales.justin at jboss.com">Ales Justin</a>
+ */
+public class AccessRestrictionTestCase extends AbstractTestCaseWithSetup
+{
+ /** The bean info factory */
+ private Configuration configuration = new BasicConfiguration();
+
+ /**
+ * Create a new ContainerTest.
+ *
+ * @param name the test name
+ */
+ public AccessRestrictionTestCase(String name)
+ {
+ super(name);
+ }
+
+ public static Test suite()
+ {
+ return suite(AccessRestrictionTestCase.class);
+ }
+
+ /**
+ * Default setup with security manager enabled
+ *
+ * @param clazz the class
+ * @return the delegate
+ * @throws Exception for any error
+ */
+ public static AbstractTestDelegate getDelegate(Class<?> clazz) throws Exception
+ {
+ AbstractTestDelegate delegate = new AbstractTestDelegate(clazz);
+ delegate.enableSecurity = true;
+ return delegate;
+ }
+
+ public void testBeanFieldAccess() throws Throwable
+ {
+ // First try to get the Bean info without the priviledge on the private field
+ FieldsClass test = new FieldsClass();
+ // This should work
+ BeanInfo beanInfo = configuration.getBeanInfo(FieldsClass.class, BeanAccessMode.ALL);
+
+ // We should be able to set the public field
+ beanInfo.setProperty(test, "pubString", "public");
+ assertEquals("public", test.pubString);
+
+ // But we should be able to set the private field
+ try
+ {
+ beanInfo.setProperty(test, "privString", "private");
+ fail("should not be here");
+ }
+ catch (Throwable t)
+ {
+ checkThrowable(AccessControlException.class, t);
+ }
+ try
+ {
+ beanInfo.getProperty(test, "privString");
+ fail("should not be here");
+ }
+ catch (Throwable t)
+ {
+ checkThrowable(AccessControlException.class, t);
+ }
+ assertNull(test.getPrivStringNotGetter());
+
+ // Repeat for the properties
+ PropertyInfo pubProp = beanInfo.getProperty("pubString");
+ test = new FieldsClass();
+
+ pubProp.set(test, "public");
+ assertEquals("public", test.pubString);
+
+ PropertyInfo privProp = beanInfo.getProperty("privString");
+ try
+ {
+ privProp.set(test, "private");
+ fail("should not be here");
+ }
+ catch (Throwable t)
+ {
+ checkThrowable(AccessControlException.class, t);
+ }
+ try
+ {
+ privProp.get(test);
+ fail("should not be here");
+ }
+ catch (Throwable t)
+ {
+ checkThrowable(AccessControlException.class, t);
+ }
+ assertNull(test.getPrivStringNotGetter());
+
+ // Now lets disable security and check we can do what we couldn't do before
+ SecurityManager sm = suspendSecurity();
+ try
+ {
+ test = new FieldsClass();
+ beanInfo.setProperty(test, "privString", "private");
+ assertEquals("private", beanInfo.getProperty(test, "privString"));
+
+ test = new FieldsClass();
+ privProp.set(test, "private");
+ assertEquals("private", privProp.get(test));
+ }
+ finally
+ {
+ resumeSecurity(sm);
+ }
+ }
+}
Modified: projects/jboss-reflect/trunk/src/tests/org/jboss/test/beaninfo/test/BeanInfoTestSuite.java
===================================================================
--- projects/jboss-reflect/trunk/src/tests/org/jboss/test/beaninfo/test/BeanInfoTestSuite.java 2008-03-21 04:18:26 UTC (rev 71121)
+++ projects/jboss-reflect/trunk/src/tests/org/jboss/test/beaninfo/test/BeanInfoTestSuite.java 2008-03-21 04:37:44 UTC (rev 71122)
@@ -44,6 +44,7 @@
suite.addTest(BeanInfoUnitTestCase.suite());
suite.addTest(BeanInfoUtilTestCase.suite());
+ suite.addTest(AccessRestrictionTestCase.suite());
return suite;
}
Modified: projects/jboss-reflect/trunk/src/tests/org/jboss/test/classinfo/test/AccessRestrictionTestCase.java
===================================================================
--- projects/jboss-reflect/trunk/src/tests/org/jboss/test/classinfo/test/AccessRestrictionTestCase.java 2008-03-21 04:18:26 UTC (rev 71121)
+++ projects/jboss-reflect/trunk/src/tests/org/jboss/test/classinfo/test/AccessRestrictionTestCase.java 2008-03-21 04:37:44 UTC (rev 71122)
@@ -25,8 +25,7 @@
import java.security.AccessControlException;
import junit.framework.Test;
-import org.jboss.beans.info.spi.BeanAccessMode;
-import org.jboss.beans.info.spi.BeanInfo;
+
import org.jboss.config.plugins.BasicConfiguration;
import org.jboss.config.spi.Configuration;
import org.jboss.reflect.plugins.introspection.ReflectFieldInfoImpl;
@@ -76,19 +75,6 @@
return delegate;
}
- protected BeanInfo getBeanInfo(Class<?> clazz, BeanAccessMode mode)
- {
- SecurityManager sm = suspendSecurity();
- try
- {
- return configuration.getBeanInfo(clazz, mode);
- }
- finally
- {
- resumeSecurity(sm);
- }
- }
-
protected ClassInfo getClassInfo(Class<?> clazz)
{
SecurityManager sm = suspendSecurity();
@@ -130,11 +116,25 @@
assertInstanceOf(t, IllegalAccessException.class);
}
- impl.setField(field); // So I'll use this hole
- field = impl.getField(); // This should have an access check
- // why does this work?!?
- field.set(tester, "foobar");
- assertEquals("foobar", tester.getPrivString());
+ try
+ {
+ impl.setField(field);
+ fail("Should not be here");
+ }
+ catch (Throwable t)
+ {
+ checkThrowable(AccessControlException.class, t);
+ }
+ try
+ {
+ field.set(tester, "foobar");
+ fail("Should not be here");
+ }
+ catch (Throwable t)
+ {
+ checkThrowable(IllegalAccessException.class, t);
+ }
+ assertNull("foobar", tester.getPrivString());
Runnable runnable = new Runnable()
{
@@ -155,13 +155,11 @@
other.start();
other.join();
// we should get an error here
-/*
assertNotNull("Should get access restriction exception.", other.getError());
RuntimeException re = assertInstanceOf(other.getError(), RuntimeException.class);
Throwable cause = re.getCause();
assertNotNull(cause);
assertInstanceOf(cause, AccessControlException.class, false);
-*/
}
public void testFieldAccessFromOther() throws Throwable
@@ -172,6 +170,8 @@
FieldInfo pub = classInfo.getDeclaredField("pubString");
assertNotNull(pub);
final FieldInfo pri = classInfo.getDeclaredField("privString");
+
+ // Shouldn't be able to set the private field
try
{
pri.set(tester, "foobar");
@@ -181,6 +181,26 @@
{
assertInstanceOf(t, AccessControlException.class);
}
+ // Shouldn't be able to get the private field
+ try
+ {
+ pri.get(tester);
+ fail("Should not be here.");
+ }
+ catch (Throwable t)
+ {
+ assertInstanceOf(t, AccessControlException.class);
+ }
+ // Shouldn't be able to steal the private field which has setAccessible(true)
+ try
+ {
+ ((ReflectFieldInfoImpl) pri).getField();
+ fail("Should not be here.");
+ }
+ catch (Throwable t)
+ {
+ assertInstanceOf(t, AccessControlException.class);
+ }
ErrorHolderThread other = new ErrorHolderThread(new Runnable()
{
public void run()
More information about the jboss-cvs-commits
mailing list