SEC01-J. Be careful using doPrivileged

The static method doPrivileged is used to affirm that the invoking method is taking responsibility for exercising its own permissions and that the access permissions of its callers should be ignored. For example, an application may have permissions to operate on a sensitive file, however, a caller of this application may be allowed to operate with only basic user permissions. Invoking doPrivileged() in the context of this method allows it to exercise its own (possibly elevated) permissions under such circumstances.

Noncompliant Code Example

There are two fallacies in this noncompliant code example. First, the doPrivileged method is being called from inside the openPasswordFile method. The openPasswordFile method is privileged and returns a FileInputStream reference to its caller. This allows any caller to call openPasswordFile() directly and obtain a reference to the sensitive file due to the inherent privileges present within the corresponding code. Second, the name of the sensitive password file is user controllable which introduces other risks such as unaccounted misuse of miscellaneous sensitive files.

class password {
 
 public static void changePassword(String password_file) throws FileNotFoundException {
    FileInputStream fin;
    fin = openPasswordFile(password_file);
 }
	
 public static FileInputStream openPasswordFile(String password_file) throws FileNotFoundException {
    //Declare as final and assign before the body of the anonymous inner class
    //Array f[] is used to maintain language semantics while using final 
    final FileInputStream f[]={null};
    final String file =  password_file;
    //Use own privilege to open the sensitive password file
    AccessController.doPrivileged(new PrivilegedAction() {
    public Object run() {   
      try {
            f[0] = new FileInputStream("c:\\" + file);	//Perform privileged action
      }catch(FileNotFoundException cnf) { System.err.println(cnf.getMessage()); }
      return null;    //Still mandatory to return from run()
     }
    });
   return f[0];  //Returns a reference to privileged objects (inappropriate)
 }	
}

Compliant Solution

The openPasswordFile method controls access to the sensitive password file and returns its reference. Since it cannot control being invoked by untrusted user methods, it should not assert its privileges within the body. Instead, changePassword() the caller method, can safely assert its own privilege whenever someone else calls it. This is because changePassword() does not return a reference to the sensitive file to any caller but processes the file internally. Also, since the name of the password file is hard-coded in the code, caller supplied (tainted) inputs are not used.

class password {

 public static void changePassword() {
  //Use own privilege to open the sensitive password file
  final String password_file = "password"; 
  final FileInputStream f[] = {null};
  AccessController.doPrivileged(new PrivilegedAction() {
  public Object run() {
   try {
         f[0] = openPasswordFile(password_file);  //call the privileged method here
   }catch(FileNotFoundException cnf) { System.err.println(cnf.getMessage()); }
   return null;
   }
  });
 //Perform other operations such as password verification
 }	

 public static FileInputStream openPasswordFile(String password_file) throws FileNotFoundException {
  FileInputStream f = new FileInputStream("c:\\" + password_file);
  //Perform read/write operations on password file
  return f;
 }
}

Risk Assessment

TODO

Automated Detection

TODO

Search for vulnerabilities resulting from the violation of this rule on the CERT website.

References

Inside Java 2 Platform Security, 6.4 AccessController
Sun Secure Coding Guidelines http://java.sun.com/security/seccodeguide.html
Documentation http://java.sun.com/j2se/1.4.2/docs/api/java/security/AccessController.html#doPrivileged(java.security.PrivilegedAction)