Bug 58750

Summary:

[WIN] Share openTemporaryFile with WinCE

Product:

WebKit

Reporter:

Patrick R. Gansterer <paroga>

Component:

New Bugs

Assignee:

Patrick R. Gansterer <paroga>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

abarth, aroben, eric, sfalken, webkit.review.bot

Priority:

Version:

528+ (Nightly build)

Hardware:

Other

OS:

OS X 10.5

Attachments:

Description	Flags
Patch	aroben: review-
Patch	none

Patrick R. Gansterer

Reported 2011-04-17 13:50:32 PDT

[WIN] Use GetTempFileNameW in openTemporaryFile

Attachments
Patch (4.23 KB, patch) 2011-04-17 13:54 PDT, Patrick R. Gansterer	aroben: review-	Details Formatted Diff Diff
Patch (3.70 KB, patch) 2012-02-20 04:28 PST, Patrick R. Gansterer	no flags	Details Formatted Diff Diff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.

Patrick R. Gansterer

Comment 1 2011-04-17 13:54:38 PDT

Created attachment 89961 [details] Patch

Steve Falkenburg

Comment 2 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.

Patrick R. Gansterer

Comment 3 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?

Eric Seidel (no email)

Comment 4 2011-04-18 09:41:45 PDT

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

Steve Falkenburg

Comment 5 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?

Patrick R. Gansterer

Comment 6 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.

Adam Roben (:aroben)

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

Patrick R. Gansterer

Comment 8 2012-02-20 04:28:21 PST

Created attachment 127806 [details] Patch

WebKit Review Bot

Comment 9 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>

WebKit Review Bot

Comment 10 2012-02-20 08:40:18 PST

All reviewed patches have been landed. Closing bug.

Note You need to log in before you can comment on or make changes to this bug.