New issue
Advanced search Search tips

Issue 801602 link

Starred by 1 user

Issue metadata

Status: Verified
Owner:
Closed: Jan 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux
Pri: 2
Type: Bug-Security



Sign in to add a comment

ASSERT: 0 <= value && value < symbolsCount

Project Member Reported by ClusterFuzz, Jan 12 2018

Issue description

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

Fuzzer: ochang_js_fuzzer
Job Type: linux_asan_d8_dbg
Platform Id: linux

Crash Type: ASSERT
Crash Address: 
Crash State:
  0 <= value && value < symbolsCount
  
Sanitizer: address (ASAN)

Regressed: https://clusterfuzz.com/revisions?job=linux_asan_d8_dbg&range=41870:41871

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

Issue filed automatically.

See https://github.com/google/clusterfuzz-tools for more information.
 
Project Member

Comment 1 by ClusterFuzz, Jan 12 2018

Labels: Test-Predator-Auto-Owner
Owner: js...@chromium.org
Status: Assigned (was: Untriaged)
Automatically assigning owner based on suspected regression changelist https://chromium.googlesource.com/v8/v8/+/a6749915d3d0167893153526de0ba61b16ab0d26 (Ship Intl.DateTimeFormat.formatToParts()).

If this is incorrect, please remove the owner and apply the Test-Predator-Wrong-CLs label.

Comment 2 by js...@chromium.org, Jan 12 2018

Hmmm.... this sounds deja vu to me.   I thought I had fixed it before. Let me take a look. 

Comment 3 by js...@chromium.org, Jan 12 2018

It's  bug 774873  where I've seen this. 
This may be another API. (due to too many redirection issue, I can't access the details, yet)

Comment 4 by js...@chromium.org, Jan 12 2018

It's  bug 774833  .  It was also DateTimeFormat.  

Comment 5 by js...@chromium.org, Jan 12 2018

Labels: -Security_Severity-High Security_Severity-Medium
    U_ASSERT(0 <= value && value < symbolsCount);
    if (0 <= value && value < symbolsCount) {
        if (monthPattern == NULL) {
            dst += symbols[value];
        } else {
            SimpleFormatter(*monthPattern, 1, 1, status).format(symbols[value], dst, status);
        }
    }


After assertion, there's a run-time check for the condition. If the condition is not met, nothing is done (there'd be no out-of-bound array access). So, the the risk is rather low. 

Comment 6 by js...@chromium.org, Jan 13 2018

1.7976931348623157e+308 is passed as |date| to DateTimeFormat. It's clamped at MAX_MILLIS (1.838821689216e+17). 

(gdb) frame 4
#4  0x00007ffff5f1efd9 in icu_60::_appendSymbolWithMonthPattern (dst=..., 
    value=-72796052, symbols=0x5555556bcb18, symbolsCount=12, monthPattern=0x0, 
    status=@0x7fffffffc64c: U_ZERO_ERROR)
    at ../../third_party/icu/source/i18n/smpdtfmt.cpp:1249
1249        U_ASSERT(0 <= value && value < symbolsCount);
(gdb) p value
$14 = -72796052

Nonetheless, an invalid month value is passed to appendSymbolWithMonthPattern.

Comment 7 by js...@chromium.org, Jan 13 2018

Calendar fields are filled with bogus values for date=MAX_MILLIS. 

= {0, -52842, -72796052, -306783357, 5, 28, -2147483506, 2, 4, 1, 4, 16, 0, 0, 
  0, -28800000, 0, -52842, 3, -52842, 2130706431, 57600000, 0}

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

When |days| is 2128757991 (corresponding to MAX_MILLIS with some offset, year and month go crazy in  IslamicCalendar::handleComputeFields(). 


612         if (cType == CIVIL || cType == TBLA) {
(gdb) n
613             if(cType == TBLA) {
(gdb) n
617             year  = (int)ClockMath::floorDivide( (double)(30 * days + 10646) , 10631.0 );
(gdb) n
618             month = (int32_t)uprv_ceil((days - 29 - yearStart(year)) / 29.5 );
(gdb) p year
$43 = -52842
(gdb) p days
$44 = 2128757991
(gdb) p yearStart(year)
$45 = -18725798
(gdb) n
619             month = month<11?month:11;
(gdb) p month
$46 = -72796052



Comment 8 by js...@chromium.org, Jan 13 2018

Components: -Blink>JavaScript Blink>JavaScript>Internationalization
There's an integer overflow at the following line for days=2128757991

year  = (int)ClockMath::floorDivide( (double)(30 * days + 10646) , 10631.0 );

casting days to int64_t solved the issue. 

Need to check all the calendars listed in 
http://www.unicode.org/repos/cldr/tags/latest/common/bcp47/calendar.xml

Comment 9 by js...@chromium.org, Jan 13 2018

I checked the following locales:

   "fa-u-ca-persian",
    "ar-u-ca-islamic-civil",
    "ar-u-ca-islamic-umalqura",
    "ar-u-ca-islamic-tbla",
    "ar-u-ca-islamic-rgsa",
    "he-u-ca-hebrew",
    "zh-u-ca-chinese",
    "ko-u-ca-dangi",
    "ja-u-ca-japanese",
    "am-u-ca-ethiopic",
    "am-u-ca-ethioaa",
    "hi-u-ca-indian",
    "th-u-ca-buddhist",

And they're all fine. 

Comment 10 by js...@chromium.org, Jan 13 2018

Status: Started (was: Assigned)
Project Member

Comment 11 by sheriffbot@chromium.org, Jan 13 2018

Labels: Pri-1
Project Member

Comment 12 by bugdroid1@chromium.org, Jan 14 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/deps/icu.git/+/899a6154c27b7c31445c25f2a2021f2716ba1960

commit 899a6154c27b7c31445c25f2a2021f2716ba1960
Author: Jungshik Shin <jshin@chromium.org>
Date: Sun Jan 14 01:57:17 2018

Fix an integer overflow in Islamic calendar

When the time is set to MAX_MILLIS, Islamic calendar
year and month calculation results in an integer overflow.

TBR=littledan@chromium.org
Bug:  801602 
Test: See the bug (v8 test will be added)
Change-Id: Ifcb6907de81de83b690f76a536818054815a15a7
Reviewed-on: https://chromium-review.googlesource.com/865524
Reviewed-by: Jungshik Shin <jshin@chromium.org>

[modify] https://crrev.com/899a6154c27b7c31445c25f2a2021f2716ba1960/README.chromium
[add] https://crrev.com/899a6154c27b7c31445c25f2a2021f2716ba1960/patches/islamcal.patch
[modify] https://crrev.com/899a6154c27b7c31445c25f2a2021f2716ba1960/source/i18n/islamcal.cpp

Comment 13 by js...@chromium.org, Jan 14 2018

Labels: -Pri-1 -Security_Severity-Medium Security_Severity-Low Pri-2
Because the checked done by assert() is also done right after assert, there's little security risk.  Anyway, a patch is landed in ICU. I'll roll ICU to a revision to have that fix. 

Project Member

Comment 14 by bugdroid1@chromium.org, Jan 17 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/c9379112632c3e8211e6aed51e8cf52a95da8993

commit c9379112632c3e8211e6aed51e8cf52a95da8993
Author: Jungshik Shin <jshin@chromium.org>
Date: Wed Jan 17 03:25:47 2018

Roll ICU to c8ca296

https://chromium.googlesource.com/chromium/deps/icu.git/+log/f3d25bc..c8ca296

$ git log f3d25bc..c8ca296 --date=short --no-merges --format='%ad %ae %s'

 2018-01-16 jshin@chromium.org Revert "SmallIntFormatter: simplify impl.."
 2018-01-13 jshin@chromium.org Fix an integer overflow in Islamic calendar
 2017-12-21 digit@google.com SmallIntFormatter: simplify implementation.

TBR=digit@chromium.org

Bug:  801602 
Test: See the bug (v8 test will be added)
Change-Id: Id5dea71ce958cfd0055b764c1a47a12c2b5aab27
Reviewed-on: https://chromium-review.googlesource.com/868278
Commit-Queue: Jungshik Shin <jshin@chromium.org>
Reviewed-by: Jungshik Shin <jshin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#529571}
[modify] https://crrev.com/c9379112632c3e8211e6aed51e8cf52a95da8993/DEPS

Comment 15 by js...@chromium.org, Jan 17 2018

Status: Fixed (was: Started)
Project Member

Comment 16 by ClusterFuzz, Jan 17 2018

ClusterFuzz has detected this issue as fixed in range 50634:50635.

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

Fuzzer: ochang_js_fuzzer
Job Type: linux_asan_d8_dbg
Platform Id: linux

Crash Type: ASSERT
Crash Address: 
Crash State:
  0 <= value && value < symbolsCount
  
Sanitizer: address (ASAN)

Regressed: https://clusterfuzz.com/revisions?job=linux_asan_d8_dbg&range=41870:41871
Fixed: https://clusterfuzz.com/revisions?job=linux_asan_d8_dbg&range=50634:50635

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

See https://github.com/google/clusterfuzz-tools 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 17 by ClusterFuzz, Jan 17 2018

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

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

Comment 18 by sheriffbot@chromium.org, Jan 17 2018

Labels: -Restrict-View-SecurityTeam Restrict-View-SecurityNotify
Project Member

Comment 19 by bugdroid1@chromium.org, Jan 27 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/v8/v8.git/+/6ecd9702269f386349ba8ba8ad97ac91c69441f8

commit 6ecd9702269f386349ba8ba8ad97ac91c69441f8
Author: Jungshik Shin <jshin@chromium.org>
Date: Sat Jan 27 10:52:46 2018

Add a test for a huge time value and month display


Bug:  chromium:801602 
Test: intl/date-format/month-far-future.js
Cq-Include-Trybots: luci.v8.try:v8_linux_noi18n_rel_ng
Change-Id: Ie2279680e8879c943cbf0873a933d2633e759212
Reviewed-on: https://chromium-review.googlesource.com/868376
Reviewed-by: Adam Klein <adamk@chromium.org>
Commit-Queue: Jungshik Shin <jshin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#50914}
[add] https://crrev.com/6ecd9702269f386349ba8ba8ad97ac91c69441f8/test/intl/date-format/month-far-future.js

Project Member

Comment 20 by sheriffbot@chromium.org, Apr 25 2018

Labels: -Restrict-View-SecurityNotify allpublic
This bug has been closed for more than 14 weeks. Removing security view restrictions.

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

Sign in to add a comment