Closed
Bug 1387937
Opened 7 years ago
Closed 7 years ago
Suspicious use-after-free at intl/icu/source/i18n/zonemeta.cpp
Categories
(Core :: JavaScript: Internationalization API, defect, P3)
Tracking
()
RESOLVED
FIXED
mozilla57
People
(Reporter: alexc, Assigned: m_kato)
References
Details
(Keywords: sec-other, Whiteboard: [adv-main57-][post-critsmash-triage])
Attachments
(2 files, 1 obsolete file)
1.01 KB,
patch
|
jfkthame
:
review+
|
Details | Diff | Splinter Review |
1.97 KB,
patch
|
jfkthame
:
review+
|
Details | Diff | Splinter Review |
User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/59.0.3071.115 Safari/537.36 Steps to reproduce: Hi, Our code scanner has reported a use-after-free issue at function `ZoneMeta::createMetazoneMappings(const UnicodeString &tzid)` of intl/icu/source/i18n/zonemeta.cpp, At the following code piece and the implementation of `deleteOlsonToMetaMappingEntry`: if (mzMappings == NULL) { mzMappings = new UVector(deleteOlsonToMetaMappingEntry, NULL, status); if (U_FAILURE(status)) { delete mzMappings; deleteOlsonToMetaMappingEntry(entry); // <== this is `uprv_free(entry)` uprv_free(entry); // `uprv_free(entry)` again ? break; } } // implementation of `deleteOlsonToMetaMappingEntry` /** * Deleter for OlsonToMetaMappingEntry */ static void U_CALLCONV deleteOlsonToMetaMappingEntry(void *obj) { icu::OlsonToMetaMappingEntry *entry = (icu::OlsonToMetaMappingEntry*)obj; uprv_free(entry); } seems there is a duplication of uprv_free? Could any one have look and see if it is a true issue? Regards, SourceBrella Inc.
Assignee | ||
Updated•7 years ago
|
See Also: → http://bugs.icu-project.org/trac/ticket/13301
Assignee | ||
Updated•7 years ago
|
Component: Untriaged → JavaScript: Internationalization API
Product: Firefox → Core
Assignee | ||
Comment 1•7 years ago
|
||
This seems to be double free, and this is 3rd party library. I mark as sec at first.
Group: core-security
Hi Kato, for the link you gave me earlier, http://bugs.icu-project.org/trac/ticket/13301 seems they haven't open for registration, and unfortunately the url needs TICKET_VIEW access right which I don't have, it the url referenced to this issue?
Updated•7 years ago
|
Group: core-security → javascript-core-security
Assignee | ||
Comment 3•7 years ago
|
||
(In reply to Alex CHEN from comment #2) > Hi Kato, > > for the link you gave me earlier, > http://bugs.icu-project.org/trac/ticket/13301 > seems they haven't open for registration, and unfortunately the url needs > TICKET_VIEW access right which I don't have, > > it the url referenced to this issue? I filed this as sensitive bug. But it is no way to create account, so I cannot check this too :-<.
Comment 4•7 years ago
|
||
Since the two frees follow each other immediately it's hard to see how an attacker could take advantage of it. maybe something on another thread could be allocating things meanwhile, but that'd be a nightmarish timing needle to thread.
Keywords: sec-low
(In reply to Daniel Veditz [:dveditz] from comment #4) > Since the two frees follow each other immediately it's hard to see how an > attacker could take advantage of it. maybe something on another thread could > be allocating things meanwhile, but that'd be a nightmarish timing needle to > thread. Does that mean this bug is confirmed but it has low security risk?
The newest version of libicu has fix this bug. It deletes the extra free statement: ``deleteOlsonToMetaMappingEntry(entry);". Thus, this bug is confirmed and fixed in the trunk of libicu.
Assignee | ||
Comment 7•7 years ago
|
||
(In reply to Alex CHEN from comment #6) > The newest version of libicu has fix this bug. It deletes the extra free > statement: ``deleteOlsonToMetaMappingEntry(entry);". > > Thus, this bug is confirmed and fixed in the trunk of libicu. Thanks, this seems to be http://bugs.icu-project.org/trac/changeset/40324
Assignee | ||
Comment 8•7 years ago
|
||
Assignee | ||
Comment 9•7 years ago
|
||
Assignee | ||
Comment 10•7 years ago
|
||
Attachment #8906504 -
Attachment is obsolete: true
Comment 11•7 years ago
|
||
Is the correct action here for us to make the same change or upgrade our libicu or something else? Thanks
Assignee | ||
Comment 12•7 years ago
|
||
(In reply to Naveed Ihsanullah [:naveed] from comment #11) > Is the correct action here for us to make the same change or upgrade our > libicu or something else? Thanks This fix will be ICU 60. This isn't critical because this double-free occurs on low memory situation only. But we should land this in m-c since ICU 60 will be next month.
Flags: needinfo?(m_kato)
Assignee | ||
Comment 13•7 years ago
|
||
Comment on attachment 8906503 [details] [diff] [review] Part 1. Import changeset 40324 This is import patch from http://bugs.icu-project.org/trac/changeset/40324. Although this is double-free issue, this isn't critical issue because new UVector has to return error status since malloc in UVector::_init is failure. If we should want until ICU60 release, please set r-.
Attachment #8906503 -
Flags: review?(jfkthame)
Assignee | ||
Updated•7 years ago
|
Attachment #8906505 -
Flags: review?(jfkthame)
Comment 14•7 years ago
|
||
Comment on attachment 8906503 [details] [diff] [review] Part 1. Import changeset 40324 Review of attachment 8906503 [details] [diff] [review]: ----------------------------------------------------------------- Although the risk here looks very low in practice, given that it's already confirmed upstream and fixed for the next release, I think it's fine to go ahead and patch this for now.
Attachment #8906503 -
Flags: review?(jfkthame) → review+
Updated•7 years ago
|
Attachment #8906505 -
Flags: review?(jfkthame) → review+
Comment 15•7 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/a9bf7b6cf6d4
Assignee: nobody → m_kato
Status: UNCONFIRMED → RESOLVED
Closed: 7 years ago
status-firefox55:
--- → wontfix
status-firefox56:
--- → wontfix
status-firefox-esr52:
--- → wontfix
Resolution: --- → FIXED
Target Milestone: --- → mozilla57
Updated•7 years ago
|
Group: javascript-core-security → core-security-release
Updated•7 years ago
|
Whiteboard: [adv-main57+]
Updated•7 years ago
|
Alias: CVE-2017-7843
Updated•7 years ago
|
Updated•7 years ago
|
Whiteboard: [adv-main57-]
Updated•7 years ago
|
Flags: qe-verify-
Whiteboard: [adv-main57-] → [adv-main57-][post-critsmash-triage]
Updated•6 years ago
|
Group: core-security-release
You need to log in
before you can comment on or make changes to this bug.
Description
•