Page History

...

Noncompliant Code Example

There are two fallacies in In this noncompliant code example. First, the doPrivileged method is being called from inside within the openPasswordFile() method. The openPasswordFile() method is privileged and returns a FileInputStream reference to its caller. This allows any an untrusted caller to call openPasswordFile() directly and obtain a reference to the sensitive file due to the inherent privileges possessed by 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.

public 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)
 }	
}

In general, when any method containing the doPrivileged block exposes a field such as a reference beyond its own boundary, it becomes trivial for untrusted callers to exploit the program.

Compliant Solution

The openPasswordFile() method controls access to the sensitive password file and returns its reference. Because it cannot control being invoked by untrusted user methodsFor this reason, it should not assert its privileges within the bodybe directly invokable. Instead, the changePassword() the caller method, can safely assert its own privilege whenever someone else calls it method must be used to forward any requests to openPasswordFile(). This is because changePassword() does not return a reference to the sensitive file to any caller but and processes the file internally. Also, Observe that caller supplied (tainted) inputs are not used because the name of the password file is hard-coded in the code, caller supplied (tainted) inputs are not used. Also, notice that the methods have been declared private in this case to limit scope.

class Password {
  private 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("Error: Operation could not be performed"); }
        return null;
      }
    });
    //Perform other operations such as password verification
  }	

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

Note that the The above compliant solution prints a general error instead of revealing sensitive information (See EXC01-J. Do not allow exceptions to transmit sensitive information). If no sensitive information can be revealed by any of the possible exceptions, an equivalent mechanism that allows exceptions to be wrapped can be used. For example, if an applet doesn't have access to read system files that contain fonts, it can accomplish the task from a privileged block without revealing any sensitive information. In fact, by not swallowing exceptions, the client will be able to deduce the symptoms of a read failure easily. In summary, if the code can throw a checked exception safely, use the form of doPrivileged method that takes a PrivilegedExceptionAction instead of a PrivilegedActionquite easily.

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

In summary, if the code can throw a checked exception safely, use the form of doPrivileged method that takes a PrivilegedExceptionAction instead of a PrivilegedAction.

Risk Assessment

Invoking doPrivileged with too large a scope may allow an attacker to perform unintended operations with elevated privilege.

Automated Detection

...

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

References

\[[API 06|AA. Java References#API 06]\] [method doPrivileged()|http://java.sun.com/javase/6/docs/api/java/security/AccessController.html#doPrivileged(java.security.PrivilegedAction)]
\[[Gong 03|AA. Java References#Gong 03]\] Sections 6.4, AccessController and 9.5 Privileged Code
\[[SCG 07|AA. Java References#SCG 07]\] Guideline 6-1 Safely invoke java.security.AccessController.doPrivileged
\[[MITRE 09|AA. Java References#MITRE 09]\] [CWE ID 266|http://cwe.mitre.org/data/definitions/266.html] "Incorrect Privilege Assignment", [CWE ID 272|http://cwe.mitre.org/data/definitions/272.html] "Least Privilege Violation" 

...