Bug 58750

Summary: [WIN] Share openTemporaryFile with WinCE
Product: WebKit Reporter: Patrick R. Gansterer <paroga>
Component: New BugsAssignee: Patrick R. Gansterer <paroga>
Status: RESOLVED FIXED    
Severity: Normal CC: abarth, aroben, eric, sfalken, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Other   
OS: OS X 10.5   
Attachments:
Description Flags
Patch
aroben: review-
Patch none

Description Patrick R. Gansterer 2011-04-17 13:50:32 PDT
[WIN] Use GetTempFileNameW in openTemporaryFile
Comment 1 Patrick R. Gansterer 2011-04-17 13:54:38 PDT
Created attachment 89961 [details]
Patch
Comment 2 Steve Falkenburg 2011-04-17 15:05:24 PDT
Comment on attachment 89961 [details]
Patch

I believe we intentionally switched away from using GetTempFileName for additional hardening in http://trac.webkit.org/changeset/34746 so I'm not sure we want to do this.
Comment 3 Patrick R. Gansterer 2011-04-17 15:44:13 PDT
(In reply to comment #2)
> (From update of attachment 89961 [details])
> I believe we intentionally switched away from using GetTempFileName for additional hardening in http://trac.webkit.org/changeset/34746 so I'm not sure we want to do this.
Before this revision there was _no_ crypto random involved. I still use the crypto api in this patch. Is there such a huge difference between 3 and 8 random characters?
Comment 4 Eric Seidel (no email) 2011-04-18 09:41:45 PDT
I'm not sure I understand the use of 3 bits of random here.
Comment 5 Steve Falkenburg 2011-04-18 09:46:07 PDT
Adding additional randomness instead of using a fixed prefix is preferable when using GetTempFileName, since otherwise it is possible to guess/iterate all combinations of the paths to temporary files.

https://buildsecurityin.us-cert.gov/bsi/articles/knowledge/coding/757-BSI.html

I'm not sure why we need to be calling GetTempFileName here though. Can we just stick with the previous larger number of random characters and change the function to generate those values assuming that CryptoAPI Win32 function isn't available on CE?
Comment 6 Patrick R. Gansterer 2011-04-18 15:03:19 PDT
(In reply to comment #5)
> Adding additional randomness instead of using a fixed prefix is preferable when using GetTempFileName, since otherwise it is possible to guess/iterate all combinations of the paths to temporary files.
> 
> https://buildsecurityin.us-cert.gov/bsi/articles/knowledge/coding/757-BSI.html
> 
> I'm not sure why we need to be calling GetTempFileName here though. Can we just stick with the previous larger number of random characters and change the function to generate those values assuming that CryptoAPI Win32 function isn't available on CE?
The problem isn't the CryptoAPI. PathCombine isn't available on WinCE.
Comment 7 Adam Roben (:aroben) 2011-04-26 16:14:18 PDT
Comment on attachment 89961 [details]
Patch

It doesn't seem good to reduce the randomness just to work around not having PathCombine. Can't we just use pathByAppendingComponent from FileSystem.h instead?
Comment 8 Patrick R. Gansterer 2012-02-20 04:28:21 PST
Created attachment 127806 [details]
Patch
Comment 9 WebKit Review Bot 2012-02-20 08:40:12 PST
Comment on attachment 127806 [details]
Patch

Clearing flags on attachment: 127806

Committed r108243: <http://trac.webkit.org/changeset/108243>
Comment 10 WebKit Review Bot 2012-02-20 08:40:18 PST
All reviewed patches have been landed.  Closing bug.