SEC21-J. Remove superfluous code from privileged blocks

Noncompliant Code Example

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;
    }
  }); // end of doPrivileged()
}

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. However, f[0] should not leak out to untrusted code (see [SEC02-J. Guard doPrivileged blocks against untrusted invocation and leakage of sensitive data]). Minimize the amount of code that requires elevated privileges; this eases the necessary task of auditing 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;
    }
  });  // end of doPrivileged()
  // Operations on the file using handle f[0]
  // Ensure that the file reference remains contained within changePassword()
}

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
[[MITRE 2009]] CWE 272

SEC00-J. Avoid granting excess privileges      02. Platform Security (SEC)      SEC12-J. Do not grant untrusted code access to classes in inaccessible packages