Versions Compared

Key

  • This line was added.
  • This line was removed.
  • Formatting was changed.

Do not operate on unvalidated or untrusted data (also known as tainted data) in a privileged block. An attacker can supply malicious input that could result in privilege escalation attacks. Appropriate mitigations include hard coding values rather than accepting arguments (when appropriate) and validating or sanitizing data before the privileged operations (see rule IDS00-J. Sanitize untrusted data passed across a trust boundary).

...

Code Block
bgColor#FFcccc
private void privilegedMethod(final String filename) 
                              throws FileNotFoundException {
  try {
    FileInputStream fis =
       (FileInputStream) AccessController.doPrivileged(
          new PrivilegedExceptionAction() {
        public FileInputStream run() throws FileNotFoundException {
          return new FileInputStream(filename);
        }
      }
    );
    // do something with the file and then close it
  } catch (PrivilegedActionException e) {
    // forward to handler and log
  }
}

...

This compliant solution invokes the cleanAFilenameAndPath()) sanitization method to disallow malicious inputs. Successful operation of the sanitization method indicates that the input is acceptable and the doPrivileged() block can be executed.

Code Block
bgColor#ccccff
private void privilegedMethod(final String filename) 
                              throws FileNotFoundException {
  final String cleanFilename;
  try {
    cleanFilename = cleanAFilenameAndPath(filename);
  } catch (/* exception as per spec of cleanAFileNameAndPath */) {
    // log or forward to handler as appropriate based on specification
    // of cleanAFilenameAndPath
  }
  try {
    FileInputStream fis =
        (FileInputStream) AccessController.doPrivileged(
          new PrivilegedExceptionAction() {
        public FileInputStream run() throws FileNotFoundException {
          return new FileInputStream(cleanFilename);
        }
      }
    );
    // do something with the file and then close it
  } catch (PrivilegedActionException e) {
    // forward to handler and log
  }
}

One potential drawback of this approach is that effective sanitization methods can be difficult to write. A benefit of this approach is that it works well in combination with taint analysis (see the Automated Detection section for this rule). For more information on how to perform secure file operations see rule FIO00-J. Do not operate on files in shared directories.

...

Sanitization of tainted inputs always carries the risk that the data is not fully sanitized. Both file and path name equivalence and directory traversal are common examples of vulnerabilities arising from the improper sanitization of path and file name inputs (see rule IDS02-J. Canonicalize path names before validating them). A design that requires an unprivileged user to access an arbitrary, protected file (or other resource) is always suspect. Consider alternatives such as using a hard coded resource name or permitting the user to select only from a list of options that are indirectly mapped to the resource names.

This compliant solution both explicitly hard codes the name of the file and also confines the variables used in the privileged block to the same method. This ensures that no malicious file can be loaded by exploiting the privileged method.

Code Block
bgColor#ccccff
static final String FILEPATH = "/path/to/protected/file/fn.ext";

private void privilegedMethod() throws FileNotFoundException {
 
  try {
    FileInputStream fis =
       (FileInputStream) AccessController.doPrivileged(
        new PrivilegedExceptionAction() {
        public FileInputStream run() throws FileNotFoundException {
          return new FileInputStream(FILEPATH);
        }
      }
    );
    // do something with the file and then close it
  } catch (PrivilegedActionException e) {
    // forward to handler and log
  }
}

...

Tools that support taint analysis enable assurance of code usage that is substantially similar to the first compliant solution. Typical taint analyses assume that a method one or more methods exist (s) that can cleanse potentially tainted inputs, providing untainted outputs (or appropriate errors). The taint analysis then ensures that only untainted data is used inside the doPrivileged block. Note that the static analyses must necessarily assume that the cleansing methods are always successful; , while in reality, this may not be the case.

Related Guidelines

MITRE CWE

CWE-266, "Incorrect Privilege Assignment" . Incorrect privilege assignment

 

CWE-272, "Least Privilege Violation" . Least privilege violation

 

CWE-732, "Incorrect Permission Assignment for Critical Resource" . Incorrect permission assignment for critical resource

Secure Coding Guidelines for the Java Programming Language, Version 3.0

Guideline 6-2. Safely invoke java.security.AccessController.doPrivileged

Bibliography

<ac:structured-macro ac:name="unmigrated-wiki-markup" ac:schema-version="1" ac:macro-id="1037d8384ed4e5e3-8adfad88-41664cfb-843fa36f-3221be932ca75b4793107e3a"><ac:plain-text-body><![CDATA[

[[API 2006

AA. Bibliography#API 06]]

[method Method doPrivileged()

http://java.sun.com/javase/6/docs/api/java/security/AccessController.html#doPrivileged(java.security.PrivilegedAction)]

]]></ac:plain-text-body></ac:structured-macro>

<ac:structured-macro ac:name="unmigrated-wiki-markup" ac:schema-version="1" ac:macro-id="c810b7ede2386cbf-3b1b32b6-4eab4213-84e284e7-cd6eb025a159bd2edc1fe334"><ac:plain-text-body><![CDATA[

[[Gong 2003

AA. Bibliography#Gong 03]]

Sections 6.4, "AccessController" AccessController, and 9.5, Privileged Code

]]></ac:plain-text-body></ac:structured-macro>

 

9.5 "Privileged Code"

<ac:structured-macro ac:name="unmigrated-wiki-markup" ac:schema-version="1" ac:macro-id="fa4339f053c45629-e40617b4-4e0c43bf-a32baa8d-5fde51111f8b95ed10fa42a0"><ac:plain-text-body><![CDATA[

[[Jovanovic 2006

AA. Bibliography#Jovanovic 06]]

" Pixy: A Static Analysis Tool for Detecting Web Application Vulnerabilities "

]]></ac:plain-text-body></ac:structured-macro>

...