FilePermission class leaks sensitive information

Somebody might consider it ironic that the security class which is responsible mapping access permission to files (java.io.FilePermission) is actually leaking information about the filesystem.

FilePermission uses a doPrivileged block to obtain a canonical path to the file/folder given as a parameter to the constructor. The canonical path is then stored in a private and transient field (called cpath) which has no accessor method.

The canonical path is security-sensitive, because if you give it the input of "." it will become the full (canonical) path to the current execution directory.

Also, in windows, if I give it a path, such as "c:\windows" it becomes "C:\WINDOWS" on my machine, as both the drive letter and windows folder are uppercase. If I give it a path that does not exist, such as "C:\whatever" it does not get altered. Thus I can test the existence of files and folders.

The cpath is not directly accessible, but the FilePermission class has a hashCode method, and the implementation is:

384  public int hashCode() {
385     return this.cpath.hashCode();
386  }


So the hashcode of the String of the canonical path is available. I looked into the possibility of reversing the string hash, but it's not really practical. The simple algorith (which is explained here) is easy to reverse, but as it's extremely lossy, the number of strings that have any given hash is very big.

File or folder existence on Windows can be easily tested by giving a toLowerCase and toUpperCase versions of any path to FilePermission and then comparing the hashcodes. If the hashcodes are equal, the file/folder exists, if they're unequal, it doesn't exist.

For example, on my machine, the following:

001 import java.io.FilePermission;
002 
003 public class FP {
004     public static void main(String[] args) throws Exception {
005         System.out.println(fileExists("C:/windows"));
006         System.out.println(fileExists("C:/filedoesnotexist"));
007     }
008     
009     static boolean fileExists(String name) {
010         return new FilePermission(name.toLowerCase(), "read").hashCode() == new FilePermission(name.toUpperCase(), "read").hashCode();
011     }
012 }


Yields the output:
true
false

In similar fashion, you could compare the hashes of FilePermissions for ".", "..", "../..", "../../.." until the hashcode stops changing, which means you've hit the root (Drive-Letter:\ on windows or / on linux/unix/etc). The depth of the execution folder can thus be determined and it is possible to try to guess each of the folders of the path individually.

It's not a very serious problem at all, but it's one I found to be amusing both for the simplicity of it and the fact that it's in the very class that is used to map access to files.

Java SE Security - Part II (Immutability)

Since I named the first bit as "Part I" in February, I'm long overdue for the second part. The problem is that my hands are tied by the fact that Sun still hasn't fixed a lot of the things I wanted to write about, so I'm short on material.

Immutability (@wikipedia)

Aside from being a software design pattern, immutability plays a big role in security, as well. At least in an environment such as the Java security sandbox. The Sun Secure Coding Guidelines section about input and ouput talks about making immutable copies of inputs and outputs.

Even if you've never put serious thought into this, the importance of immutability quickly becomes obvious.

Steve: Let's say my method takes a (java.util) List of Strings as a parameter, performs some security validation on the list and then performs some privileged action on the list.

001 public class PrivilegedClass {
002
003     public void processStrings(java.util.List<String> strings) {
004         for (String str : strings) {
005             if (!isOk(str)) {
006                 throw new SecurityException();
007             }
008         }
009
010         for (String str : strings) {
011             doPrivilegedOperation(str);
012         }
013     }
014
015     private void doPrivilegedOperation(String str) {
016         // privileged stuff
017         // code omitted
018     }
019
020     private boolean isOk(String str) {
021         // security check for string
022         // code omitted
023         return false;
024     }
025
026 }


Bob: The problem is that java.util.List is in interface and can be implemented in a mutable way. Also virtually all the public List implementations included in Java are mutable, so one wouldn't even have to go through the trouble of creating a mutable version - one could just use java.util.ArrayList.

The problem with the mutability is that an ill intentioned caller of the method could pass a list which contains different values while the method does its validation, and then another set of harmful values for the actual processing after the validation has passed. This could be done via timing, or by crafting one's own List implementation for a more exact attack. If the method uses .iterator() to iterate through the list, the first call to .iterator() (used by validation) could be made to return one set of Strings and the 2nd call to .iterator() (used by the post-validation processing) could be made to return another set of Strings.

It's a well known, generic problem.

Let's look at some ways to protect your method and what could go wrong with them. Forgive the silliness of some of these examples, it's just to give the notion of how easy it is to get it wrong.

Steve: I could change the method signature to only accept a more specialized StevesImmutableList.

003     public void processStrings(StevesImmutableList strings) {


Steve: I'd then define StevesImmutableList as a class whose constructor receives a String array and stores it in a private String array field and only has a getter method which returns the array.

001 package version1;
002
003 public class StevesImmutableList {
004
005     private String[] strings;
006
007     public StevesImmutableList(String[] strings) {
008         this.strings = strings;
009     }
010
011     public String[] getStrings() {
012         return this.strings;
013     }
014
015 }


Bob: Arrays are always mutable. The size can't be changed, but the contents (given its size is greater than zero) are free game. And your method is returning a reference to its internal array.

Steve: What if I define my private internal array as final?

Bob: That doesn't help. It'll guarantee that your private field always points to the same array, but the array contents are free game.

Steve: Better have the method call .clone() on the array and return a copy of the array instead of the internal array.

001 package version2;
002
003 public class StevesImmutableList {
004
005     private String[] strings;
006
007     public StevesImmutableList(String[] strings) {
008         this.strings = strings;
009     }
010
011     public String[] getStrings() {
012         return this.strings.clone();
013     }
014
015 }


Bob: Your constructor is receiving an array and storing it as the internal array. A caller with bad intentions could create an array, keep a reference to it, pass it to your object and later on modify it.

Steve: Ok, let's call .clone() on the incoming array as well.

001 package version3;
002
003 public class StevesImmutableList {
004
005     private String[] strings;
006
007     public StevesImmutableList(String[] strings) {
008         this.strings = strings.clone();
009     }
010
011     public String[] getStrings() {
012         return this.strings.clone();
013     }
014
015 }


Steve: Let's also add an overloaded constructor that receives a List to improve the usability of this class.

001 package version4;
002
003 public class StevesImmutableList {
004
005     private String[] strings;
006
007     public StevesImmutableList(java.util.List<String> stringList) {
008         this.strings = stringList.toArray(new String[stringList.size()]);
009     }
010
011     public StevesImmutableList(String[] strings) {
012         this.strings = strings.clone();
013     }
014
015     public String[] getStrings() {
016         return this.strings.clone();
017     }
018
019 }


Bob: Now you're calling toArray() on a List object that you can't really trust. It could return an array and keep a reference to that array for itself for future modification.

Steve: Ok, let's do a manual conversion of the List to an array.

001 package version5;
002
003 public class StevesImmutableList {
004
005     private String[] strings;
006
007     public StevesImmutableList(java.util.List<String> stringList) {
008         this.strings = new String[stringList.size()];
009         for (int i = 0; i < stringList.size(); i++) {
010             this.strings[i] = stringList.get(i);
011         }
012     }
013
014     public StevesImmutableList(String[] strings) {
015         this.strings = strings.clone();
016     }
017
018     public String[] getStrings() {
019         return this.strings.clone();
020     }
021
022 }


Steve: It'd also be nice to be able to serialize this object for passing between JVMs and/or storing object state somewhere. So let's make the class implement Serializable.

001 package version6;
002
003 public final class StevesImmutableList implements java.io.Serializable {
004
005     private String[] strings;
006
007     public StevesImmutableList(java.util.List<String> stringList) {
008         this.strings = new String[stringList.size()];
009         for (int i = 0; i < stringList.size(); i++) {
010             this.strings[i] = stringList.get(i);
011         }
012     }
013
014     public StevesImmutableList(String[] strings) {
015         this.strings = strings.clone();
016     }
017
018     public String[] getStrings() {
019         return this.strings.clone();
020     }
021
022 }


Bob: Serial data can be constructed in such a way that when read, will contain a String array, followed by a StevesImmutableList containing the same String array in the private field, thus making the object mutable.

Steve: Let's make the private field transient and create a readObject method which reads the array from the stream and then clones it before assigning it to the field. Also create a compatible writeObject method.

001 package version7;
002
003 public final class StevesImmutableList implements java.io.Serializable {
004
005     private transient String[] strings;
006
007     public StevesImmutableList(java.util.List<String> stringList) {
008         this.strings = new String[stringList.size()];
009         for (int i = 0; i < stringList.size(); i++) {
010             this.strings[i] = stringList.get(i);
011         }
012     }
013
014     public StevesImmutableList(String[] strings) {
015         this.strings = strings.clone();
016     }
017
018     public String[] getStrings() {
019         return this.strings.clone();
020     }
021
022     private void readObject(java.io.ObjectInputStream s)
023             throws java.io.IOException, ClassNotFoundException {
024         String[] streamStrings = (String[]) s.readObject();
025         this.strings = streamStrings.clone();
026     }
027
028     private void writeObject(java.io.ObjectOutputStream s)
029             throws java.io.IOException {
030         s.writeObject(this.strings);
031     }
032 }


Now some examples from core Java classes

java.lang.reflection.Proxy

Proxy has a method (getProxyClass) which takes a ClassLoader and an array of Class objects, that are supposed to be interfaces, as parameters. It performs some validation on the Class array, and then it dynamically defines and returns a new Class for a dynamic Proxy class which implements all the interfaces of the Class array. If the validation fails, the method throws an Exception (and obviously doesn't return anything).

The getProxyClass uses the user-suplied Class array for the validation and later on for the construction of the Proxy class, so it could be called with an array which has one set of classes for the validation and another set of classes for the Proxy construction.

However, the validation here, as far as I can tell, is strictly for usability. Without the validation, if you gave the method an invalid set of parameters it would sometimes fail with some strange class verification exception. With the validation, the caller gets feedback which is more readily understandable.

java.lang.String

Strings are supposed to be immutable. A lot of things depend on the immutability of Strings. There is no defensive programming in the core classes against mutable Strings (which makes sense, because it'd take a lot of work).

The immutability of Strings hinges on the fact that one cannot access the char array in which the String contents are stored. Now, String does leak the internal array to any registered CharSet who wants it. However, registering a CharSet requires a CharSetProvider and that, in turn, requires the charsetProvider permission.

Unsigned applets don't have that permission and can't create their own registered CharSets. But it is something to keep in mind. If you're granting the charsetProvider permission, you're pretty much implicitly giving away full access.