Martin,

Your fix looks good to me, just need a test case to putback. Should be pretty straightforward to create a custom SecurityManager that throws a ACE instead of a SE during a checkRead(), and then link together.

Brad


On 8/21/2013 3:51 PM, Martin Buchholz wrote:
Adding Alexey Utkin, who appears to be the author of the lines I am
proposing to modify.  Alexey, you are invited to take ownership of this fix.


On Wed, Aug 21, 2013 at 3:43 PM, Martin Buchholz <marti...@google.com
<mailto:marti...@google.com>> wrote:

    Hi security team,

    There's some code in ProcessBuilder.java to avoid leaking data in
    case ProcessBuilder.start fails.
    It appears to have an obvious bug, with an obvious fix.

    
http://cr.openjdk.java.net/~martin/webrevs/openjdk8/ProcessBuilder-checkRead/

    checkRead is spec'ed to throw SecurityException, not
    AccessControlException. If checkRead does throw SecurityException,
    then start will throw the wrong exception.

    Untested.

    @@ -1033,9 +1033,9 @@
                      // Can not disclose the fail reason for read-protected 
files.
                      try {
                          security.checkRead(prog);
    -                } catch (AccessControlException ace) {
    +                } catch (SecurityException e) {
                          exceptionInfo = "";
    -                    cause = ace;
    +                    cause = e;
                      }
                  }
                  // It's much easier for us to create a high-quality error


Reply via email to