Bug 135463

Summary: Fix unintentional integer overflow before widen
Product: WebKit Reporter: przemeks@pkuczynski.com <p.kuczynski>
Component: WebKit2Assignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: bunhere, cdumez, commit-queue, gyuyoung.kim, japhet, m.leszko, mpakulavelrutka, sergio
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
porposed patch none

Description przemeks@pkuczynski.com 2014-07-31 07:08:55 PDT
Overflowing expression is evaluated using operands arithmetic but then is used in
context which expects an wider integer type. To avoid overflow at least one operand
has to be representative of the wider type.
Comment 1 przemeks@pkuczynski.com 2014-07-31 07:14:19 PDT
Created attachment 235828 [details]
porposed patch
Comment 2 WebKit Commit Bot 2014-07-31 07:18:27 PDT
Attachment 235828 [details] did not pass style-queue:


ERROR: Source/WebCore/loader/FTPDirectoryParser.cpp:508:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Total errors found: 1 in 6 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 3 przemeks@pkuczynski.com 2014-07-31 07:21:06 PDT
(In reply to comment #2)
> Attachment 235828 [details] did not pass style-queue:
> 
> 
> ERROR: Source/WebCore/loader/FTPDirectoryParser.cpp:508:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
> Total errors found: 1 in 6 files
> 
> 
> If any of these errors are false positives, please file a bug against check-webkit-style.

Whole file is written using 2-space indent.
Comment 4 Darin Adler 2014-08-18 13:53:53 PDT
Comment on attachment 235828 [details]
porposed patch

View in context: https://bugs.webkit.org/attachment.cgi?id=235828&action=review

> Source/WebCore/loader/FTPDirectoryParser.cpp:508
> -              uint64_t size = strtoul(tokens[1], NULL, 10) * 512;
> +              uint64_t size = strtoull(tokens[1], 0, 10) * 512;

The style checker told you to change the NULL to a 0, but really we would like to use nullptr here.

> Source/WebCore/loader/ProgressTracker.cpp:237
> +    estimatedBytesForPendingRequests = static_cast<long long>(progressItemDefaultEstimatedLength) * numPendingOrLoadingRequests;

A better change would be to change type of progressItemDefaultEstimatedLength from int to long long instead of adding the cast here.
Comment 5 WebKit Commit Bot 2014-08-18 14:14:49 PDT
Comment on attachment 235828 [details]
porposed patch

Clearing flags on attachment: 235828

Committed r172725: <http://trac.webkit.org/changeset/172725>
Comment 6 WebKit Commit Bot 2014-08-18 14:14:54 PDT
All reviewed patches have been landed.  Closing bug.