Bug 134351 - [Win] Fix potential buffer overrun in DLLLauncher
Summary: [Win] Fix potential buffer overrun in DLLLauncher
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC All
: P2 Normal
Assignee: Brent Fulgham
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2014-06-26 10:44 PDT by Brent Fulgham
Modified: 2014-06-26 17:04 PDT (History)
7 users (show)

See Also:


Attachments
Patch (1.13 KB, patch)
2014-06-26 10:46 PDT, Brent Fulgham
no flags Details | Formatted Diff | Diff
Patch (1.42 KB, patch)
2014-06-26 10:47 PDT, Brent Fulgham
no flags Details | Formatted Diff | Diff
Patch (1.33 KB, patch)
2014-06-26 16:55 PDT, Brent Fulgham
darin: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Brent Fulgham 2014-06-26 10:44:41 PDT
Correct a possible buffer overrun in the DLLLauncher utility.
Comment 1 Brent Fulgham 2014-06-26 10:45:02 PDT
<rdar://problem/17469185>
Comment 2 Brent Fulgham 2014-06-26 10:46:41 PDT
Created attachment 233914 [details]
Patch
Comment 3 Brent Fulgham 2014-06-26 10:47:19 PDT
Created attachment 233915 [details]
Patch
Comment 4 Darin Adler 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';
Comment 5 Brent Fulgham 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.
Comment 6 Brent Fulgham 2014-06-26 16:55:59 PDT
Created attachment 233946 [details]
Patch
Comment 7 Brent Fulgham 2014-06-26 17:04:26 PDT
Committed r170510: <http://trac.webkit.org/changeset/170510>