The java.security.AccessController class is part of Java's security mechanism; it is responsible for enforcing the applicable security policy. This class's static doPrivileged() method executes a code block with a relaxed security policy. The doPrivileged() method stops permissions from being checked further down the call chain.

Consequently, any method that invokes doPrivileged() must assume responsibility for enforcing its own security on the code block supplied to doPrivileged(). Likewise, code in the doPrivileged() method must not leak sensitive information or capabilities.

For example, suppose that a web application must maintain a sensitive password file for a web service and also must run untrusted code. The application could then enforce a security policy preventing the majority of its own code—as well as all untrusted code—from accessing the sensitive file. Because it must also provide mechanisms for adding and changing passwords, it can call the doPrivileged() method to temporarily allow untrusted code to access the sensitive file. In this case, any privileged block must prevent all information about passwords from being accessible to untrusted code.

Noncompliant Code Example

In this noncompliant code example, the doPrivileged() method is called from the openPasswordFile() method. The openPasswordFile() method is privileged and returns a FileInputStream for the sensitive password file. Because the method is public, it could be invoked by an untrusted caller.

public class PasswordManager {

  public static void changePassword() throws FileNotFoundException {
    FileInputStream fin = openPasswordFile();

    // Test old password with password in file contents; change password,
    // then close the password file
  }

  public static FileInputStream openPasswordFile()
      throws FileNotFoundException {
    final String password_file = "password";
    FileInputStream fin = null;
    try {
      fin = AccessController.doPrivileged(
        new PrivilegedExceptionAction<FileInputStream>() {
          public FileInputStream run() throws FileNotFoundException {
            // Sensitive action; can't be done outside privileged block
            FileInputStream in = new FileInputStream(password_file);
            return in;
          }
      });
    } catch (PrivilegedActionException x) {
      Exception cause = x.getException();
      if (cause instanceof FileNotFoundException) {
        throw (FileNotFoundException) cause;
      } else {
        throw new Error("Unexpected exception type", cause); 
      }
    }
    return fin;
  }
}

Compliant Solution

In general, when any method containing a privileged block exposes a field (such as an object reference) beyond its own boundary, it becomes trivial for untrusted callers to exploit the program. This compliant solution mitigates the vulnerability by declaring openPasswordFile() to be private. Consequently, an untrusted caller can call changePassword() but cannot directly invoke the openPasswordFile() method.

public class PasswordManager {
  public static void changePassword() throws FileNotFoundException {
    // ...
  }

  private static FileInputStream openPasswordFile() 
     throws FileNotFoundException {
    // ...
  }
}

Compliant Solution (Hiding Exceptions)

The previous noncompliant code example and the previous compliant solution throw a FileNotFoundException when the password file is missing. If the existence of the password file is itself considered sensitive information, this exception also must be prevented from leaking outside the trusted code.

This compliant solution suppresses the exception, leaving the array to contain a single null value to indicate that the file does not exist. It uses the simpler PrivilegedAction class rather than PrivilegedExceptionAction to prevent exceptions from propagating out of the doPrivileged() block. The Void return type is recommended for privileged actions that do not return any value.

class PasswordManager {

  public static void changePassword() {
    FileInputStream fin = openPasswordFile();
    if (fin == null) {
      // No password file; handle error
    }

    // Test old password with password in file contents; change password
  }

  private static FileInputStream openPasswordFile() {
    final String password_file = "password";
    final FileInputStream fin[] = { null };
    AccessController.doPrivileged(new PrivilegedAction<Void>() {
        public Void run() {
          try {
            // Sensitive action; can't be done outside
            // doPrivileged() block
            fin[0] = new FileInputStream(password_file);
          } catch (FileNotFoundException x) {
            // Report to handler
          }
          return null;
        }
    });
    return fin[0];
  }
}

Risk Assessment

Returning references to sensitive resources from within a doPrivileged() block can break encapsulation and confinement and can leak capabilities. Any caller who can invoke the privileged code directly and obtain a reference to a sensitive resource or field can maliciously modify its elements.

Rule

Severity

Likelihood

Remediation Cost

Priority

Level

SEC00-J

Medium

Likely

High

P6

L2

Automated Detection

Identifying sensitive information requires assistance from the programmer; fully automated identification of sensitive information is beyond the current state of the art.

Assuming user-provided tagging of sensitive information, escape analysis could be performed on the doPrivileged() blocks to prove that nothing sensitive leaks out from them. Methods similar to those used in thread-role analysis could be used to identify the methods that must, or must not, be called from doPrivileged() blocks.

Related Guidelines

MITRE CWE

CWE-266, Incorrect Privilege Assignment
CWE-272, Least Privilege Violation

Secure Coding Guidelines for Java SE, Version 5.0

Guideline 9-3 / ACCESS-3: Safely invoke java.security.AccessController.doPrivileged

Android Implementation Details

The java.security package exists on Android for compatibility purposes only, and it should not be used.

Bibliography

[API 2014]

Method doPrivileged()

[Gong 2003]

Section 6.4, "AccessController"
Section 9.5, "Privileged Code"

 


14 Comments

  1. Using the return value from doPrivileged return value is better than using a hack to return a value. The examples swallow exceptions, but a form of doPrivileged allows exception to be wrapped.

    public static void changePassword() throws FileNotFoundException {
      //Use own privilege to open the sensitive password file
      final String password_file = "password"; 
      try \{
        final InputStream in =
          AccessController.doPrivileged(new PrivilegedExceptionAction<InputStream>() {
            public InputStream run() {
              return openPasswordFile(password_file);  //call the privileged method here
            }
          });
          //Perform other operations such as password verification
        } catch (PrivilegedActionException exc) {
          Exception cause = exc.getException();
          if (cause instanceof FileNotFoundException) {
            throw (FileNotFoundException)cause;
          } else {
            throw new Error(
              "Unexpected exception type",
               cause
            );
          }
        }
      });
    }
    
    1. The return value hack may be removed, it was just there to demonstrate how one can return things from doPrivileged when the return value is not a single statement/method call.

      For exception wrapping, I could not compile the above code. I had to add a throws to public InputStream run() throws FileNotFoundException and comment out });. Is this desired or am I missing something? I suspect this would again swallow exceptions.

      public static void changePassword() throws FileNotFoundException {
        //Use own privilege to open the sensitive password file
        final String password_file = "password";
        try {
              final InputStream in =
              AccessController.doPrivileged(new PrivilegedExceptionAction<InputStream>() {
      		  
      	  public InputStream run() throws FileNotFoundException {
      	    return openPasswordFile(password_file); //call the privileged method here
       	  }
      	});
             //Perform other operations such as password verification
            } catch (PrivilegedActionException exc) {
      		
               Exception cause = exc.getException();
               if (cause instanceof FileNotFoundException) {
       	   throw (FileNotFoundException)cause;
               } else {
      	   throw new Error(
      	   "Unexpected exception type",cause);
                 }
      	 }
      		//});
        }
      
      }
      

      On a sidenote, starting and ending tags {code} can be used to denote source code...

      1. Sorry, yes the run method does need to declare any exceptions within the method. It could be just declared as throws Exception, but that isn't very helpful. In PrivilegedExceptionAction it is declared as

        T run() throws Exception;

        .

        1. I added a CS as per your suggestion, though with one modification - only throw the exception(s) when no sensitive information (such as file path) can be revealed in the process.

  2. Fred Long and I observe that the text of this guideline discusses avoiding leaks of sensitive information out of doPrivileged blocks. But the title of the guideline talks about guarding against untrusted invocation -- a totally different question.  Should we move the text to a different guideline, or should we change the title?

    1. I was just having similar thoughts. My take on this guideline is that it is really about "capabilities" and that it should be titled "SEC02-J. Guard doPrivileged blocks against untrusted invocation". The example used to illustrate the risks of untrusted invocation is leaking of sensitive information, but the way the rule is currently worded it does come across too strongly as if this were the reason for the rule.

      1. For doPrivileged code, untrusted invocations are very much fine but the code having the particular permissions (and exercising doPrivileged) should ensure that any untrusted code is unable to directly manipulate it (this includes returning handles etc). Some public wrapper (changePassword()) should be used which can achieve the desired functionality such as changing the password as an "independent task". The leaking of sensitive information or direct manipulation of sensitive files is an example and should not be in the title.

        I think the second CS is just informative. It just demonstrates the use of PrivilegedExceptionAction instead of a PrivilegedAction. It can be summed up in a line without having to go into gory details.

  3. I don't see any logging in the logging exceptions compliant solution, so I'm not at all sure what is going on there.

    There are a lot of other disconnects in this rule, to the point where I'm thinking it should be discarded. The intro paragraph lacks normative text except in the title. The "example" in the intro talks about an apparently different problem.

    1. Bumped back up to review2. The intro is clearer, and the code now supports the intro. (previously it was trying to show off PrivilegedAction and PrivilegedExceptionAction, which is informative, but unnecessary to the actual rule.)

  4. Thomas Hawtin says:

    SEC00-J In the second compliant solution, the privileged block could just return the FileInputStream. (Even if you didn't, the raw type should generally be avoided, by convention use Void for unused generic parameters.)

    1. For production code, I agree: returning the FileInputStream is a fine alternative to filling a final array...that coding practice is showcased in an earlier code sample. When I reviewed this rule several months ago, I decided to leave the 2nd CS filling the final array instead, to show that it is a perfectly fine (though distinct) coding practice.

      Wrt generics, we do have a rule against relying on raw types. I've modified the CS to use Void, as suggested.

  5. In the second CCE, where is the "Void" (not void) defined in the code?

    1. The Void datatype is part of the Java standard library, and it was created specifically to address this problem, see:

      http://download.oracle.com/javase/6/docs/api/java/lang/Void.html

      1. Thanks for the info, David! Domo.