[security-dev 00359]: Re: SecurityException in AnnotationInvocationHandler.getMemberMethods

[Sean Mullan]

I looked at the code changes and it seems fine to me.

--Sean

Joe Darcy wrote:

Hello.

On 10/16/08 12:46 PM, Martin Buchholz wrote:

Hi all,

This is a bug report with fix.
Joe Darcy, please file a bug and review this change,

I've filed 6761678 "(ann) SecurityException in AnnotationInvocationHandler.getMemberMethods" for this issue. The problem seems similar to 6370080 "(ann) Method.getAnnotations() sometimes throw SecurityException: doPrivileged or javadoc missing?," which was fixed in JDK 6 and a JDK 5 update release.

Martin, can you, Toby, and Josh review any other uses of reflection in the src/share/classes/sun/reflect/annotation package for similar problems so we can address any other such issues now?

I've looked over the change and the use of getMemberMethods in equalsImpl and don't see a problem. However, I'd like the security team to give it a once over too; security-dev folk, please take a look at this.

Thanks,

-Joe

and perhaps provide a small test case (it is impractical
to share the test we have at Google).

Description:

sun/reflect/annotation/AnnotationInvocationHandler.java.getMemberMethods
might throw if there is a security manager that does not allow
getDeclaredMethods.

The author of this code (Josh Bloch) confirms that the intent was for the
doPrivileged block in this method to prevent security exceptions.
The methods cannot escape to untrusted code.

Evaluation:

Yes.  Fix provided courtesy of Toby Reyelts and Josh Bloch at Google.

# HG changeset patch
# User martin
# Date 1224185752 25200
# Node ID 68730f05449cd4f39ce1cb82adc6c4e57f87554f
# Parent  214ebdcf7252d4862449fe0ae295e6c60a127315
SecurityException in AnnotationInvocationHandler.getMemberMethods
Summary: Move call to getDeclaredMethods inside doPrivileged
Reviewed-by:
Contributed-by: [EMAIL PROTECTED]

diff --git a/src/share/classes/sun/reflect/annotation/AnnotationInvocationHandler.java b/src/share/classes/sun/reflect/annotation/AnnotationInvocationHandler.java --- a/src/share/classes/sun/reflect/annotation/AnnotationInvocationHandler.java +++ b/src/share/classes/sun/reflect/annotation/AnnotationInvocationHandler.java

@@ -272,14 +272,14 @@
      */
     private Method[] getMemberMethods() {
         if (memberMethods == null) {
-            final Method[] mm = type.getDeclaredMethods();
-            AccessController.doPrivileged(new PrivilegedAction<Void>() {
-                public Void run() {
-                    AccessibleObject.setAccessible(mm, true);
-                    return null;
-                }
-            });
-            memberMethods = mm;
+            memberMethods = AccessController.doPrivileged(
+                new PrivilegedAction<Method[]>() {
+                    public Method[] run() {
+                        final Method[] mm = type.getDeclaredMethods();
+                        AccessibleObject.setAccessible(mm, true);
+                        return mm;
+                    }
+                });
         }
         return memberMethods;
     }