Bug 58750 – [WIN] Share openTemporaryFile with WinCE

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Patrick R. Gansterer]

[WIN] Use GetTempFileNameW in openTemporaryFile

[Patrick R. Gansterer]

Created attachment 89961 [details]
Patch

[Steve Falkenburg]

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.

[Patrick R. Gansterer]

(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?

[Eric Seidel (no email)]

I'm not sure I understand the use of 3 bits of random here.

[Steve Falkenburg]

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?

[Patrick R. Gansterer]

(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.

[Adam Roben (:aroben)]

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?

[Patrick R. Gansterer]

Created attachment 127806 [details]
Patch

[WebKit Review Bot]

Comment on attachment 127806 [details]
Patch

Clearing flags on attachment: 127806

Committed r108243: <http://trac.webkit.org/changeset/108243>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.