Bug 17030 - Small buffer overflow within initialization
Summary: Small buffer overflow within initialization
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC Linux
: P2 Normal
Assignee: Alexey Proskuryakov
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2008-01-27 03:46 PST by Stephan Binner
Modified: 2008-02-27 09:14 PST (History)
2 users (show)

See Also:


Attachments
Same as in original post (390 bytes, patch)
2008-01-27 11:32 PST, Stephan Binner
ap: review-
Details | Formatted Diff | Diff
proposed fix (1.42 KB, patch)
2008-02-21 22:33 PST, Alexey Proskuryakov
sam: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Stephan Binner 2008-01-27 03:46:18 PST
The patch should say enough :-)....

--- JavaScriptCore/kjs/date_object.cpp  2008/01/16 12:24:15     1.1
+++ JavaScriptCore/kjs/date_object.cpp  2008/01/16 12:24:21
@@ -908,7 +908,7 @@
     // fall back to local timezone
     if (!haveTZ) {
         GregorianDateTime t;
-        memset(&t, 0, sizeof(tm));
+        memset(&t, 0, sizeof(t));
         t.monthDay = day;
         t.month = month;
         t.year = year - 1900;
Comment 1 Alexey Proskuryakov 2008-01-27 07:31:10 PST
Wow, thanks for catching this!

AFAICT, on the Mac, these structs are the same - is struct tm bigger on Linux? Also, GregorianDateTime is initialized to zero anyway, so we should probably just take out the calls to memset here and elsewhere.

Would you be willing to submit this for review as described in <http://webkit.org/coding/contributing.html>?
Comment 2 Stephan Binner 2008-01-27 11:31:21 PST
On x86_64 it is.
Comment 3 Stephan Binner 2008-01-27 11:32:28 PST
Created attachment 18723 [details]
Same as in original post
Comment 4 Alexey Proskuryakov 2008-01-27 12:05:14 PST
Comment on attachment 18723 [details]
Same as in original post

Instead of correcting memset usage, we should just remove it (not just here, but all the instances that were mistakenly used for GregorianDateTime initialization).

Also, the patch needs a change log.
Comment 5 Alexey Proskuryakov 2008-02-21 22:33:51 PST
Created attachment 19273 [details]
proposed fix
Comment 6 Alexey Proskuryakov 2008-02-27 09:14:34 PST
Committed revision 30625.