SEI CERT Oracle Coding Standard for Java

Space shortcuts

Page tree

Versions Compared

Old Version 31

changes.mady.by.user Pranjal Jumde

Saved on

compared with

New Version 32

changes.mady.by.user David Svoboda

Saved on

Key

  • This line was added.
  • This line was removed.
  • Formatting was changed.
Comment: standardized error handling

...

Code Block
bgColor#FFcccc
public void processFile_nce(String filename){
  // Identify a file by its path
  Path file1 = Paths.get(filename);
 
 
  // Open the file for writing
  try(BufferedWriter bw = new BufferedWriter(new OutputStreamWriter(Files.newOutputStream(file1)))) {
      // Write to file...
  } catch (ExceptionIOException e) {
    System.out.println("Exception during file access" + e);// handle error
  } 
  
  // Close the file
  
  /*
   * A race condition here allows for an attacker to switch
   * out the file for another
   */

  // Reopen the file for reading
  Path file2 = Paths.get(filename);
    
    
  try(BufferedReader br = new BufferedReader(new InputStreamReader(Files.newInputStream(file2)))){
      String line;
      while ((line = br.readLine()) != null) {
          System.out.println(line);
      }
  } catch (ExceptionIOException e) {
    //  System.out.println("Exception during file access" + e);handle error
  } 
}

There is no guarantee that the file opened for reading is the same file that was opened for writing. An attacker can replace the original file (for example, with a symbolic link) between the first call to close() and the subsequent creation of the BufferedReader.

...

Code Block
bgColor#FFcccc
public void sameFile_nce(String filename){
  // Identify a file by its path
  Path file1 = Paths.get(filename);
  
  // Open the file for writing
  try(BufferedWriter bw = new BufferedWriter(new OutputStreamWriter(Files.newOutputStream(file1)))) {
    // Write to file
  } catch (ExceptionIOException e) {
    //  System.out.println("Exception during file access" + e);handle error
  } 
  
  // ...
  // Reopen the file for reading
  Path file2 = Paths.get(filename);
  if (!Files.isSameFile(file1, file2)) {
    System.out.println("File tampered with");
    // Handlehandle error
  }
  
  try(BufferedReader br = new BufferedReader(new InputStreamReader(Files.newInputStream(file2)))) { 
    String line;
    while ((line = br.readLine()) != null) {
      System.out.println(line);
    }
  } catch (ExceptionIOException e) {
    //  System.out.println("Exception during file access" + e);handle error
  } 
}

Unfortunately, there is no guarantee that the method isSameFile() really checks that the files are the same file. The Java 7 API for isSameFile() says:

...

Code Block
bgColor#ccccff
public void sameFile_cs(String filename) throws IOException{
  // Identify a file by its path
  Path file1 = Paths.get(filename);
  BasicFileAttributes attr1 = Files.readAttributes(file1, BasicFileAttributes.class);
  FileTime creation1 = attr1.creationTime();
  FileTime modified1 = attr1.lastModifiedTime();

  // Open the file for writing
  try (BufferedWriter bw = new BufferedWriter(new OutputStreamWriter(Files.newOutputStream(file1)))) {
    // Write to file...
  } catch (ExceptionIOException e) {
    System.out.println("Exception during file access" + e);// handle error
  } 
  
  // Reopen the file for reading
  Path file2 = Paths.get(filename);
  BasicFileAttributes attr2 = Files.readAttributes(file2, BasicFileAttributes.class);
  FileTime creation2 = attr2.creationTime();
  FileTime modified2 = attr2.lastModifiedTime();
  if ( (!creation1.equals(creation2)) || (!modified1.equals(modified2)) ) {
    System.out.println("File tampered with");
    // Handlehandle error
  }
  
  try(BufferedReader br = new BufferedReader( new InputStreamReader(Files.newInputStream(file2)))){
    String line;
    while ((line = br.readLine()) != null) {
      System.out.println(line);
    }
  } catch (ExceptionIOException e) {
    //  System.out.println("Exception during file access" + e);handle error
  } 
}

Although this solution is reasonably secure, a determined attacker could create a symbolic link with the same creation and last-modified times as the original file. Also, a time-of-check, time-of-use (TOCTOU) race condition occurs between the time the file's attributes are first read and the time the file is first opened. Likewise, a second TOCTOU condition occurs the second time the attributes are read and the file is reopened.

...

Code Block
bgColor#ccccff
public void filekey_cs(String filename) throws IOException{
  // Identify a file by its path
  Path file1 = Paths.get(filename);
  BasicFileAttributes attr1 = Files.readAttributes(file1, BasicFileAttributes.class);
  Object key1 = attr1.fileKey();
  // Open the file for writing
  try(BufferedWriter bw = new BufferedWriter(new OutputStreamWriter(Files.newOutputStream(file1)))) {
          // Write to file
  } catch (ExceptionIOException e) {
    //  System.out.println("Exception during file access" + e);handle error
  } 
  
  // Reopen the file for reading
  Path file2 = Paths.get(filename);
  BasicFileAttributes attr2 = Files.readAttributes(file2, BasicFileAttributes.class);
  Object key2 = attr2.fileKey();

  if ( !key1.equals(key2) ) {
    System.out.println("File tampered with");
    // Handlehandle error
  }

  try(BufferedReader br = new BufferedReader(new InputStreamReader(Files.newInputStream(file2)))) {
    String line;
    while ((line = br.readLine()) != null) {
      System.out.println(line);
    }
  } catch (ExceptionIOException e) {
    System.out.println("Exception during file access" + e);// handle error
  } 
}

This approach will not work on all platforms. For example, on an Intel Core i5-2400 machine running Windows 7 Enterprise, all fileKey attributes are null.

...

Code Block
bgColor#ffcccc
langjava
static long goodSize = 1024;

public void doSomethingWithFile(String filename) {
  long size = new File( filename).length();
  if (size != goodSize) {
    System.out.println("File is wrong size!");
    return;
  }

  try (BufferedReader br = new BufferedReader(new InputStreamReader(new FileInputStream( filename)))) {
    // ... work with file
  } catch (ExceptionIOException e) {
    //  System.out.println("Exception during file access" + e);handle error
  } 
}

This code is subject to a (TOCTOU) race condition between when the file size is learned and when the file is opened. If an attacker replaces a 1024-byte file with another file during this race window, they can cause this program to open any file, defeating the check.

...

Code Block
bgColor#ccccff
langjava
static long goodSize = 1024;

public void doSomethingWithFile(String filename) {
  try (FileInputStream in = new FileInputStream( filename);
    BufferedReader br = new BufferedReader(new InputStreamReader(in))) {
    long size = in.getChannel().size();
    if (size != goodSize) {
      System.out.println("File is wrong size!");
      return;
    }

    String line;
    while ((line = br.readLine()) != null) {
      System.out.println(line);
    }
  } catch (ExceptionIOException e) {
    //  System.out.println("Exception during file access" + e);handle error
  } 
}

Applicability

Many file-related vulnerabilities are exploited to cause a program to access an unintended file. Proper file identification is necessary to prevent exploitation.

...