SEC21-J. Remove superfluous code from privileged blocks

Noncompliant Code Example (Superfluous Code in doPrivileged Block)

This noncompliant code example shows a changePassword() method that attempts to open a password file using a doPrivileged block. The doPrivileged block also contains logic that operates on the file and a superfluous System.loadLibrary() call.

public void changePassword() {
  final FileInputStream f[] = { null };

  AccessController.doPrivileged(new PrivilegedAction() {
    public Object run() {
      try {
        String passwordFile = System.getProperty("user.dir") + File.separator
            + "PasswordFileName";
        f[0] = new FileInputStream(passwordFile);                                                     
        // Operations on the file ...
        System.loadLibrary("LibName");
      } catch (FileNotFoundException cnf) {
        // Forward to handler
      }
      return null;
    }
  });
}

This is a violation of the principle of least privilege because a caller who does not have the required privileges can indirectly load the library provided the security policy allows doing so. This transfers the burden of ensuring security to the administrator who implements the security policy.

Compliant Solution

This compliant solution removes the call to System.loadLibrary(). Any operations on the file descriptor f[0] must also occur outside the privileged block to make it easier to audit privileged code.

public void changePassword() {
  final FileInputStream f[] = { null };

  AccessController.doPrivileged(new PrivilegedAction() {
    public Object run() {
      try {
        String passwordFile = System.getProperty("user.dir") + File.separator
            + "PasswordFileName";
        f[0] = new FileInputStream(passwordFile);                                                     
      } catch (FileNotFoundException cnf) {
        // Forward to handler
      }
      return null;
    }
  });
  // Operations on the file ...
}

Furthermore, f[0] should not leak out to untrusted code. (Refer to guideline [SEC02-J. Guard doPrivileged blocks against untrusted invocations].) It is critical to keep the amount of code that requires elevated privileges to a minimum and audit privileged code carefully.

Risk Assessment

Failure to follow the principle of least privilege can lead to privilege escalation.

Automated Detection

Automated checking is clearly not possible in the general case. We might be able to do something with escape analysis to check that we are not leaking privileged data provided that privileged data is marked by the user, and even that would be difficult.

Related Vulnerabilities

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

Bibliography

[[API 2006]] Class java.security.AccessController
[[McGraw 2000]]
[[MITRE 2009]] CWE 272

02. Platform Security (SEC)      02. Platform Security (SEC)      SEC01-J. Minimize the accessibility of classes and their members