New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 642853 link

Starred by 1 user

Issue metadata

Status: Duplicate
Merged: issue 796807
Owner:
Closed: Dec 2017
Cc:
EstimatedDays: ----
NextAction: ----
OS: Linux
Pri: 1
Type: Bug



Sign in to add a comment

Undefined-shift in uprv_decNumberFromString_56

Project Member Reported by ClusterFuzz, Aug 31 2016

Issue description

Detailed report: https://cluster-fuzz.appspot.com/testcase?key=6407791101345792

Fuzzer: libfuzzer_icu_number_format_fuzzer
Job Type: libfuzzer_chrome_ubsan
Platform Id: linux

Crash Type: Undefined-shift
Crash Address: 
Crash State:
  uprv_decNumberFromString_56
  icu_56::DigitList::set
  icu_56::DecimalFormat::subparse
  
Regressed: https://cluster-fuzz.appspot.com/revisions?job=libfuzzer_chrome_ubsan&range=415587:415619

Minimized Testcase (0.01 Kb):
Download: https://cluster-fuzz.appspot.com/download/AMIfv94U3bpnRwK0tvJ6VPIJrWZjDbPpaw8xC4s4M10xHiBmg4pFaYc0V9kLvlQgxzZsUpnnFmIFiLJdhe8ePAdDvwPNwBiDiPRDTOpC4uDS3exu8DQ6uMGn5N54qo3e1DYSjXuDU5EdIhNlY8P0sMof21tgKEN-XA?testcase_id=6407791101345792
4E07100000080


Issue manually filed by: mummareddy

See https://chromium.googlesource.com/chromium/src/+/master/testing/libfuzzer/reproducing.md for more information.
 
Components: Blink>Internals>WTF
Labels: findit-wrong M-55 Te-Logged
Owner: aizatsky@chromium.org
Status: Assigned (was: Untriaged)
From findit tool:
Author: Mike Aizatsky
Project: chromium-icu
Changelist: https://chromium.googlesource.com/chromium/deps/icu.git/+/06c914d938e16bb4779c78209f9843954ba448de
Time: Wed May 18 21:11:34 2016
The CL last changed line 26 of file icu_number_format_fuzzer.cc, which is stack frame 5.

aizatsky@, could you please take a look and reassign if it is not related your changes. 
Owner: mummare...@chromium.org
The fuzzer is the tool that uncovers bugs in the library. Please stop assigning bugs to fuzzer authors.
Owner: js...@chromium.org
Sure, thank you.
Assigning to the owner of "//src/third_party/icu/" file jshin@. could you please take a look and reassign if it is not related your changes.

Comment 4 by tkent@chromium.org, Sep 5 2016

Components: -Blink>Internals>WTF

Comment 5 by js...@chromium.org, Sep 8 2016

Line 630 of decNumber.c ( https://cs.chromium.org/chromium/src/third_party/icu/source/i18n/decNumber.c?rcl=0&l=630 ) 

has an 'undefined behavior' because X10(e) is expanded to |(e<<3) + (e<<1)| for e >= (2^31-1 / 8). 

The author of the code lets it overflow intentionally. See lines 636 ~ 638. [1] 

    626       for (; *c=='0' && *(c+1)!='\0';) c++;  /* strip insignificant zeros  */
    627       firstexp=c;                            /* save exponent digit place  */
    628       for (; ;c++) {
    629         if (*c<'0' || *c>'9') break;         /* not a digit  */
    630         exponent=X10(exponent)+(Int)*c-(Int)'0';
    631         } /* c  */
    632       /* if not now on a '\0', *c must not be a digit  */
    633       if (*c!='\0') break;
    634 
    635       /* (this next test must be after the syntax checks)  */
    636       /* if it was too long the exponent may have wrapped, so check  */
    637       /* carefully and set it to a certain overflow if wrap possible  */
    638       if (c>=firstexp+9+1) {
    639         if (c>firstexp+9+1 || *firstexp>'1') exponent=DECNUMMAXE*2;
    640         /* [up to 1999999999 is OK, for example 1E-1000000998]  */
    641         }
    642       if (nege) exponent=-exponent;     /* was negative  */
    643       status=0;                         /* is OK  */
    644       } /* stuff after digits  */

Lines 136 ~ 142 have ( https://cs.chromium.org/chromium/src/third_party/icu/source/i18n/decNumber.c?rcl=0&l=136 ) :

    136 /* 4. Exponent checking is minimized by allowing the exponent to      */
    137 /*    grow outside its limits during calculations, provided that      */
    138 /*    the decFinalize function is called later.  Multiplication and   */
    139 /*    division, and intermediate calculations in exponentiation,      */
    140 /*    require more careful checks because of the risk of 31-bit       */
    141 /*    overflow (the most negative valid exponent is -1999999997, for  */
    142 /*    a 999999999-digit number with adjusted exponent of -999999999). */

-------------------------

I wonder if something similar to http://bugs.icu-project.org/trac/changeset/38782 (casting) would work for ubsan. 


Comment 6 by js...@chromium.org, Sep 8 2016

Cc: aheninger@google.com
Project Member

Comment 7 by sheriffbot@chromium.org, Nov 22 2016

Labels: -Restrict-View-EditIssue
Removing EditIssue view restrictions from ClusterFuzz filed bugs. If you believe that this issue should still be restricted, please reapply the label.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot

Comment 8 by js...@chromium.org, May 16 2017

Cc: msrchandra@chromium.org js...@chromium.org
 Issue 674020  has been merged into this issue.

Comment 9 by mmoroz@chromium.org, Oct 24 2017

For more information, please see https://chromium.googlesource.com/chromium/src/+/master/testing/libfuzzer/reference.md.

The link referenced in the description is no longer valid.

(bulk edit)
Project Member

Comment 10 by ClusterFuzz, Dec 20 2017

ClusterFuzz has detected this issue as fixed in range 525156:525174.

Detailed report: https://clusterfuzz.com/testcase?key=6407791101345792

Fuzzer: libFuzzer_icu_number_format_fuzzer
Job Type: libfuzzer_chrome_ubsan
Platform Id: linux

Crash Type: Undefined-shift
Crash Address: 
Crash State:
  uprv_decNumberFromString_59
  icu_59::DigitList::set
  icu_59::DecimalFormat::subparse
  
Sanitizer: undefined (UBSAN)

Regressed: https://clusterfuzz.com/revisions?job=libfuzzer_chrome_ubsan&range=415587:415619
Fixed: https://clusterfuzz.com/revisions?job=libfuzzer_chrome_ubsan&range=525156:525174

Reproducer Testcase: https://clusterfuzz.com/download?testcase_id=6407791101345792

See https://chromium.googlesource.com/chromium/src/+/master/testing/libfuzzer/reference.md for more information.

If you suspect that the result above is incorrect, try re-doing that job on the test case report page.
Project Member

Comment 11 by ClusterFuzz, Dec 20 2017

Labels: ClusterFuzz-Verified
Status: Verified (was: Assigned)
ClusterFuzz testcase 6407791101345792 is verified as fixed, so closing issue as verified.

If this is incorrect, please add ClusterFuzz-Wrong label and re-open the issue.

Comment 12 by js...@chromium.org, Dec 23 2017

Mergedinto: 796807
Status: Duplicate (was: Verified)

Sign in to add a comment