Permission Bug in AtomicLongFieldUpdater and AtomicIntegerFieldUpdater

David Holmes David.Holmes at oracle.com
Thu Apr 15 17:49:35 PDT 2010


Hi Martin,

If this proceeds I think you need to include AtomicReferenceFieldUpdater 
in this as well.

But this is not a clear cut issue (security never is!).

If I understand the test program correctly the problem arises when the 
target object's class was loaded by a different class-loader to the 
class doing the AtomicXXXFieldUpdater creation. This seems reasonable as 
getDeclaredField states:

     SecurityException - If a security manager, s, is present and any of 
the following conditions is met:
         * invocation of s.checkMemberAccess(this, Member.DECLARED) 
denies access to the declared field
         * the caller's class loader is not the same as or an ancestor 
of the class loader for the current class and invocation of 
s.checkPackageAccess() denies access to the package of this class

So here we would not have package access. What puzzles me is then why 
our following explicit checks pass:

   sun.reflect.misc.ReflectUtil.ensureMemberAccess(
                   caller, tclass, null, modifiers);
   sun.reflect.misc.ReflectUtil.checkPackageAccess(tclass);

It seems inconsistent that getDeclaredField's internal checks say you 
don't have permission to access this field, while what should amount to 
the same checks via ReflectUtil say you do. The question is: which check 
is wrong?

David
-----


Martin Buchholz said the following on 04/16/10 08:34:
> Hi java.util.concurrent security team,
> 
> People are using Atomic field updaters to update fields in classes in
> other classloaders.
> 
> Toby writes:
> 
> We received a bug report for App Engine that AtomicLongFieldUpdater
> (and its sibling) were failing with RuntimePermission
> accessDeclaredMembers. Looking at the code, it appears that it is a
> bug for that permission to be required. AtomicLongFieldUpdater (and
> sibling) are already performing subsequent access checks that ensure
> that the caller isn't accessing a field it shouldn't be. I've attached
> a patch which moves the field lookups into doPrivileged blocks.
> 
> ...
> 
> There was a bug was filed by ehcache folks. I could follow up with
> them to see if I could get some pointers to actual usage, if you're
> looking for motivational reasoning.
> 
> ---
> 
> http://code.google.com/p/googleappengine/issues/detail?id=2769
> 
> GAE Bug Submitter writes:
> 
> AtomicLongFieldUpdater is broken in the production sandbox.
> 
> The simple test application that I wrote (atomically increments and prints
> a volatile long on each get request) works perfectly in the development
> environment.  In production creating the field updater fails with an
> AccessControlException when the internal CASUpdater tries to do reflection
> (on a user class) in order to get a reference to the volatile field.
> 
> I've attached the test app which works perfectly in development but fails
> in production.
> 
> This issue is linked to an associated Ehcache issue:
> 
> https://jira.terracotta.org/jira/browse/EHC-617
> 
> I have a modified experimental version of Toby's patch against openjdk7 here:
> 
> http://cr.openjdk.java.net/~martin/webrevs/openjdk7/Atomic-security/
> 
> I'm afraid to touch this security stuff myself,
> especially where we might be loosening permissions,
> but in principle I agree with Toby.
> (Even though atomic field updaters were designed to be
> used within a module (whatever that is))
> 
> The doPrivileged should not be too costly, as it's
> only performed at updater creation time.
> 
> Martin



More information about the security-dev mailing list