Bug 134351

Summary: [Win] Fix potential buffer overrun in DLLLauncher
Product: WebKit Reporter: Brent Fulgham <bfulgham>
Component: Tools / TestsAssignee: Brent Fulgham <bfulgham>
Status: RESOLVED FIXED    
Severity: Normal CC: bfulgham, bunhere, cdumez, commit-queue, ddkilzer, gyuyoung.kim, sergio
Priority: P2 Keywords: InRadar
Version: 528+ (Nightly build)   
Hardware: PC   
OS: All   
Attachments:
Description Flags
Patch
none
Patch
none
Patch darin: review+

Brent Fulgham
Reported 2014-06-26 10:44:41 PDT
Correct a possible buffer overrun in the DLLLauncher utility.
Attachments
Patch (1.13 KB, patch)
2014-06-26 10:46 PDT, Brent Fulgham 		no flags
DetailsFormatted DiffDiff
Patch (1.42 KB, patch)
2014-06-26 10:47 PDT, Brent Fulgham 		no flags
DetailsFormatted DiffDiff
Patch (1.33 KB, patch)
2014-06-26 16:55 PDT, Brent Fulgham 		darin: review+
DetailsFormatted DiffDiff
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.
Brent Fulgham
Comment 1 2014-06-26 10:45:02 PDT
<rdar://problem/17469185>
Brent Fulgham
Comment 2 2014-06-26 10:46:41 PDT
Created attachment 233914 [details] Patch
Brent Fulgham
Comment 3 2014-06-26 10:47:19 PDT
Created attachment 233915 [details] Patch
Darin Adler
Comment 4 2014-06-26 11:41:15 PDT
Comment on attachment 233915 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=233915&action=review > Tools/win/DLLLauncher/DLLLauncherMain.cpp:177 > if (len >= bufSize) > - len = bufSize - 1; > + len = bufSize - 2; This change looks wrong. > Tools/win/DLLLauncher/DLLLauncherMain.cpp:179 > errorMessage[len + 1] = 0; This is the line of code that seems to have a bug. It should say errorMessage[len] = '\0';
Brent Fulgham
Comment 5 2014-06-26 16:54:17 PDT
Comment on attachment 233915 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=233915&action=review >> Tools/win/DLLLauncher/DLLLauncherMain.cpp:179 >> errorMessage[len + 1] = 0; > > This is the line of code that seems to have a bug. It should say errorMessage[len] = '\0'; You are right. I just read the documents on FormatMessage, and it returns the number of characters NOT including null. So len + 1 was always wrong, as it gave at least one wchar_t's worth of garbage at the end of the line.
Brent Fulgham
Comment 6 2014-06-26 16:55:59 PDT
Created attachment 233946 [details] Patch
Brent Fulgham
Comment 7 2014-06-26 17:04:26 PDT
Committed r170510: <http://trac.webkit.org/changeset/170510>
Note You need to log in before you can comment on or make changes to this bug.