You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@harmony.apache.org by "Imran Ghory (JIRA)" <ji...@apache.org> on 2007/08/22 00:16:30 UTC

[jira] Created: (HARMONY-4663) File.createTempFile() is insecure

File.createTempFile() is insecure
---------------------------------

                 Key: HARMONY-4663
                 URL: https://issues.apache.org/jira/browse/HARMONY-4663
             Project: Harmony
          Issue Type: Bug
          Components: Classlib
            Reporter: Imran Ghory


createTempFile() generates  a random file name by calling   genTempFile(prefix, newSuffix, tmpDirFile), however that function generates it's randomness by calling new java.util.Random().nextInt(); which creates a Random() object seeded with the current time. This makes it predictable and thus insecure[1].


[1] See section "7.10.1.2. Temporary Files" at  http://www.faqs.org/docs/Linux-HOWTO/Secure-Programs-HOWTO.html


-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


Re: [jira] Created: (HARMONY-4663) File.createTempFile() is insecure

Posted by Tim Ellison <t....@gmail.com>.
Mikhail Loenko wrote:
> 2007/8/24, Tim Ellison <t....@gmail.com>:
>> Leo Li wrote:
>>> On 8/23/07, Tim Ellison <t....@gmail.com> wrote:
>>>> Please take a look at the comments on:
>>>>   https://issues.apache.org/jira/browse/HARMONY-4663
>>>>
>>>> While I don't think it will make much difference to most people, is
>>>> there any objection to making the following change so that temp file
>>>> names are less predictable?
>>>    Agree, although I am also not sure there is urgent demand for adopting
>>> SecureRandom.
>> True, but it is a trivial change, so if nobody objects then I'll go
>> ahead and do it.
> 
> I think it's worth doing

Thanks Leo/Mikhail.  Done at r569338.

Regards,
Tim


Re: [jira] Created: (HARMONY-4663) File.createTempFile() is insecure

Posted by Mikhail Loenko <ml...@gmail.com>.
2007/8/24, Tim Ellison <t....@gmail.com>:
> Leo Li wrote:
> > On 8/23/07, Tim Ellison <t....@gmail.com> wrote:
> >> Please take a look at the comments on:
> >>   https://issues.apache.org/jira/browse/HARMONY-4663
> >>
> >> While I don't think it will make much difference to most people, is
> >> there any objection to making the following change so that temp file
> >> names are less predictable?
> >
> >    Agree, although I am also not sure there is urgent demand for adopting
> > SecureRandom.
>
> True, but it is a trivial change, so if nobody objects then I'll go
> ahead and do it.

I think it's worth doing

Thanks,
Mikhail

>
> Regards,
> Tim
>
>

Re: [jira] Created: (HARMONY-4663) File.createTempFile() is insecure

Posted by Tim Ellison <t....@gmail.com>.
Leo Li wrote:
> On 8/23/07, Tim Ellison <t....@gmail.com> wrote:
>> Please take a look at the comments on:
>>   https://issues.apache.org/jira/browse/HARMONY-4663
>>
>> While I don't think it will make much difference to most people, is
>> there any objection to making the following change so that temp file
>> names are less predictable?
> 
>    Agree, although I am also not sure there is urgent demand for adopting
> SecureRandom.

True, but it is a trivial change, so if nobody objects then I'll go
ahead and do it.

Regards,
Tim


Re: [jira] Created: (HARMONY-4663) File.createTempFile() is insecure

Posted by Leo Li <li...@gmail.com>.
On 8/23/07, Tim Ellison <t....@gmail.com> wrote:
>
> Please take a look at the comments on:
>   https://issues.apache.org/jira/browse/HARMONY-4663
>
> While I don't think it will make much difference to most people, is
> there any objection to making the following change so that temp file
> names are less predictable?


   Agree, although I am also not sure there is urgent demand for adopting
SecureRandom.

Regards,
> Tim
>
>
> Index: modules/luni/src/main/java/java/io/File.java
> ===================================================================
> --- modules/luni/src/main/java/java/io/File.java        (revision 568557)
> +++ modules/luni/src/main/java/java/io/File.java        (working copy)
> @@ -1158,7 +1158,7 @@
>
>     private static File genTempFile(String prefix, String suffix, File
> directory) {
>         if (counter == 0) {
> -            int newInt = new java.util.Random().nextInt();
> +            int newInt = new java.security.SecureRandom().nextInt();
>             counter = ((newInt / 65535) & 0xFFFF) + 0x2710;
>         }
>         StringBuilder newName = new StringBuilder();
>
>
>


-- 
Leo Li
China Software Development Lab, IBM

Re: [jira] Created: (HARMONY-4663) File.createTempFile() is insecure

Posted by Tim Ellison <t....@gmail.com>.
Please take a look at the comments on:
   https://issues.apache.org/jira/browse/HARMONY-4663

While I don't think it will make much difference to most people, is
there any objection to making the following change so that temp file
names are less predictable?

Regards,
Tim


Index: modules/luni/src/main/java/java/io/File.java
===================================================================
--- modules/luni/src/main/java/java/io/File.java	(revision 568557)
+++ modules/luni/src/main/java/java/io/File.java	(working copy)
@@ -1158,7 +1158,7 @@

     private static File genTempFile(String prefix, String suffix, File
directory) {
         if (counter == 0) {
-            int newInt = new java.util.Random().nextInt();
+            int newInt = new java.security.SecureRandom().nextInt();
             counter = ((newInt / 65535) & 0xFFFF) + 0x2710;
         }
         StringBuilder newName = new StringBuilder();



[jira] Resolved: (HARMONY-4663) [classlib][luni]File.createTempFile() is insecure

Posted by "Tim Ellison (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/HARMONY-4663?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Tim Ellison resolved HARMONY-4663.
----------------------------------

    Resolution: Fixed

Thanks Imran.

Fixed in LUNI module at repo revision r569338.

Please check it was fixed as you expected.


> [classlib][luni]File.createTempFile() is insecure
> -------------------------------------------------
>
>                 Key: HARMONY-4663
>                 URL: https://issues.apache.org/jira/browse/HARMONY-4663
>             Project: Harmony
>          Issue Type: Bug
>          Components: Classlib
>            Reporter: Imran Ghory
>            Assignee: Tim Ellison
>
> createTempFile() generates  a random file name by calling   genTempFile(prefix, newSuffix, tmpDirFile), however that function generates it's randomness by calling new java.util.Random().nextInt(); which creates a Random() object seeded with the current time. This makes it predictable and thus insecure[1].
> [1] See section "7.10.1.2. Temporary Files" at  http://www.faqs.org/docs/Linux-HOWTO/Secure-Programs-HOWTO.html

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Closed: (HARMONY-4663) [classlib][luni]File.createTempFile() is insecure

Posted by "Tim Ellison (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/HARMONY-4663?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Tim Ellison closed HARMONY-4663.
--------------------------------


No response, assuming it is ok.

> [classlib][luni]File.createTempFile() is insecure
> -------------------------------------------------
>
>                 Key: HARMONY-4663
>                 URL: https://issues.apache.org/jira/browse/HARMONY-4663
>             Project: Harmony
>          Issue Type: Bug
>          Components: Classlib
>            Reporter: Imran Ghory
>            Assignee: Tim Ellison
>
> createTempFile() generates  a random file name by calling   genTempFile(prefix, newSuffix, tmpDirFile), however that function generates it's randomness by calling new java.util.Random().nextInt(); which creates a Random() object seeded with the current time. This makes it predictable and thus insecure[1].
> [1] See section "7.10.1.2. Temporary Files" at  http://www.faqs.org/docs/Linux-HOWTO/Secure-Programs-HOWTO.html

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Updated: (HARMONY-4663) [classlib][luni]File.createTempFile() is insecure

Posted by "Tim Ellison (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/HARMONY-4663?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Tim Ellison updated HARMONY-4663:
---------------------------------

    Assignee: Tim Ellison
     Summary: [classlib][luni]File.createTempFile() is insecure  (was: File.createTempFile() is insecure)

> [classlib][luni]File.createTempFile() is insecure
> -------------------------------------------------
>
>                 Key: HARMONY-4663
>                 URL: https://issues.apache.org/jira/browse/HARMONY-4663
>             Project: Harmony
>          Issue Type: Bug
>          Components: Classlib
>            Reporter: Imran Ghory
>            Assignee: Tim Ellison
>
> createTempFile() generates  a random file name by calling   genTempFile(prefix, newSuffix, tmpDirFile), however that function generates it's randomness by calling new java.util.Random().nextInt(); which creates a Random() object seeded with the current time. This makes it predictable and thus insecure[1].
> [1] See section "7.10.1.2. Temporary Files" at  http://www.faqs.org/docs/Linux-HOWTO/Secure-Programs-HOWTO.html

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Commented: (HARMONY-4663) File.createTempFile() is insecure

Posted by "Paulex Yang (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/HARMONY-4663?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_12522364 ] 

Paulex Yang commented on HARMONY-4663:
--------------------------------------

OK, I agree to use SecureRandom instead. 

About the Random() constructor, it seems RI (Sun JDK 5) has changed the behavior to use only current time as seed, at least doesn't depend System.currentTimeMillis() only any more. see test below:

import java.util.*;
public class RandomTest{
    public static void main(String[] args){
        long time = System.currentTimeMillis();
        Random r1 = new Random();
        long time2 = System.currentTimeMillis();
        System.out.println("If at same millis second? "+(time==time2));
        Random r2 = new Random(time);
        System.out.println("first int with default constructor: "+r1.nextInt());
        System.out.println("first int with explicit seed of current time: "+r2.nextInt());
    }
}

Generally it outputs:

If at same millis second? true
first int with default constructor: 1243876523
first int with explicit seed of current time: 1252033812






> File.createTempFile() is insecure
> ---------------------------------
>
>                 Key: HARMONY-4663
>                 URL: https://issues.apache.org/jira/browse/HARMONY-4663
>             Project: Harmony
>          Issue Type: Bug
>          Components: Classlib
>            Reporter: Imran Ghory
>
> createTempFile() generates  a random file name by calling   genTempFile(prefix, newSuffix, tmpDirFile), however that function generates it's randomness by calling new java.util.Random().nextInt(); which creates a Random() object seeded with the current time. This makes it predictable and thus insecure[1].
> [1] See section "7.10.1.2. Temporary Files" at  http://www.faqs.org/docs/Linux-HOWTO/Secure-Programs-HOWTO.html

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Commented: (HARMONY-4663) File.createTempFile() is insecure

Posted by "Paulex Yang (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/HARMONY-4663?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_12521666 ] 

Paulex Yang commented on HARMONY-4663:
--------------------------------------

Imran, 

Thanks for finding this and the wonderful link. IMHO this is a java.util.Random issue more than java.io.File, because we cannot get much control on the File's permission from Java, unless we rewrite it in JNI with native OS support. 

About the Random, currently Harmony's default constructor for Random use the sum of current time and hashcode(inherited behavior from Object and generally internal address of this object) as seed, do you think it can resolve this issue?  

Or any suggestions from you on this? Thanks a lot.

> File.createTempFile() is insecure
> ---------------------------------
>
>                 Key: HARMONY-4663
>                 URL: https://issues.apache.org/jira/browse/HARMONY-4663
>             Project: Harmony
>          Issue Type: Bug
>          Components: Classlib
>            Reporter: Imran Ghory
>
> createTempFile() generates  a random file name by calling   genTempFile(prefix, newSuffix, tmpDirFile), however that function generates it's randomness by calling new java.util.Random().nextInt(); which creates a Random() object seeded with the current time. This makes it predictable and thus insecure[1].
> [1] See section "7.10.1.2. Temporary Files" at  http://www.faqs.org/docs/Linux-HOWTO/Secure-Programs-HOWTO.html

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Commented: (HARMONY-4663) File.createTempFile() is insecure

Posted by "Imran Ghory (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/HARMONY-4663?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_12521901 ] 

Imran Ghory commented on HARMONY-4663:
--------------------------------------

hmm acccording to the 1.4.2 api spec the default Random() constructor _has to_ seed itself using just the time, although this requirement seems to have disappeared in 1.5 onwards,  but I can't find anything in the 1.5 changelog to explain this change.

But for this bug I think we should resolve it by replacing Random with java.security.SecureRandom algorithm.

> File.createTempFile() is insecure
> ---------------------------------
>
>                 Key: HARMONY-4663
>                 URL: https://issues.apache.org/jira/browse/HARMONY-4663
>             Project: Harmony
>          Issue Type: Bug
>          Components: Classlib
>            Reporter: Imran Ghory
>
> createTempFile() generates  a random file name by calling   genTempFile(prefix, newSuffix, tmpDirFile), however that function generates it's randomness by calling new java.util.Random().nextInt(); which creates a Random() object seeded with the current time. This makes it predictable and thus insecure[1].
> [1] See section "7.10.1.2. Temporary Files" at  http://www.faqs.org/docs/Linux-HOWTO/Secure-Programs-HOWTO.html

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.