Bug 186757

Summary: Properly check the sscanf return value
Product: WebKit Reporter: Tomas Popela <tpopela>
Component: WebKitGTKAssignee: Tomas Popela <tpopela>
Status: RESOLVED FIXED    
Severity: Normal CC: bugs-noreply, calvaris, cgarcia, darin
Priority: P2    
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch none

Description Tomas Popela 2018-06-18 04:13:59 PDT 
Found by Coverity scan:

5. webkitgtk-2.20.3/Source/WebCore/page/linux/ResourceUsageThreadLinux.cpp:61: check_return: Calling "sscanf" without checking return value (as is done elsewhere 10 out of 12 times).
6. webkitgtk-2.20.3/Source/JavaScriptCore/jsc.cpp:2099: example_checked: Example 1: "sscanf(timeoutString, "%lf", &s_desiredTimeout)" has its value checked in "sscanf(timeoutString, "%lf", &s_desiredTimeout) != 1".
7. webkitgtk-2.20.3/Source/WTF/wtf/NumberOfCores.cpp:55: example_checked: Example 2: "sscanf(coresEnv, "%u", &numberOfCores)" has its value checked in "sscanf(coresEnv, "%u", &numberOfCores) == 1".
8. webkitgtk-2.20.3/Source/WTF/wtf/linux/MemoryFootprintLinux.cpp:86: example_checked: Example 3: "sscanf(buffer, "Private_Dirty: %lu", &privateDirtyInKB)" has its value checked in "sscanf(buffer, "Private_Dirty: %lu", &privateDirtyInKB) == 1".
9. webkitgtk-2.20.3/Source/JavaScriptCore/runtime/Options.cpp:250: example_assign: Example 4: Assigning: "scanResult" = return value from "sscanf(p, " %u:%u", &this->m_lowLimit, &this->m_highLimit)".
10. webkitgtk-2.20.3/Source/JavaScriptCore/runtime/Options.cpp:252: example_checked: Example 4 (cont.): "scanResult" has its value checked in "scanResult".
11. webkitgtk-2.20.3/Source/JavaScriptCore/runtime/Options.cpp:107: example_checked: Example 5: "sscanf(string, "%lf", value)" has its value checked in "sscanf(string, "%lf", value) == 1".
#    59|       unsigned long long ioWait, irq, softIrq, steal, guest, guestnice;
#    60|       ioWait = irq = softIrq = steal = guest = guestnice = 0;
#    61|->     sscanf(buffer, "cpu  %16llu %16llu %16llu %16llu %16llu %16llu %16llu %16llu %16llu %16llu",
#    62|           &userTime, &niceTime, &systemTime, &idleTime, &ioWait, &irq, &softIrq, &steal, &guest, &guestnice);
#    63|
Comment 1 Tomas Popela 2018-06-18 04:15:51 PDT 
Created attachment 342924 [details]
Patch
Comment 2 Tomas Popela 2018-06-18 06:43:33 PDT 
Comment on attachment 342924 [details]
Patch

Clearing flags on attachment: 342924

Committed r232929: <https://trac.webkit.org/changeset/232929>
Comment 3 Tomas Popela 2018-06-18 06:43:39 PDT 
All reviewed patches have been landed.  Closing bug.
Comment 4 Darin Adler 2018-06-23 10:09:45 PDT 
Comment on attachment 342924 [details]
Patch

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

> Source/WebCore/page/linux/ResourceUsageThreadLinux.cpp:64
> +    if (retVal < 10 || retVal == EOF) {

I think this should just be:

    if (retVal != 10)
Comment 5 Tomas Popela 2018-06-24 21:26:28 PDT 
(In reply to Darin Adler from comment #4)
> I think this should just be:
> 
>     if (retVal != 10)

That's true Darin! I will change it.
Comment 6 Tomas Popela 2018-06-25 03:23:03 PDT 
Committed r233142: <https://trac.webkit.org/changeset/233142>