Security: use-after-free in AttachFilteredEvent on event_bindings.cc
Reported by
jinmot...@gmail.com,
Jul 2 2016
|
||||||||||||||||
Issue description
VULNERABILITY DETAILS
When processing smart pointers in AddEventMatcher(event_filters.cc), it don't std::move the smart pointer on certain condition, like (see <--)
On line 56
EventFilter::MatcherID EventFilter::AddEventMatcher(
const std::string& event_name,
std::unique_ptr<EventMatcher> matcher <-- unique_ptr) {
MatcherID id = next_id_++;
URLMatcherConditionSet::Vector condition_sets;
if (!CreateConditionSets(id, matcher.get(), &condition_sets))
return -1; <-- frees the EventMatcher matcher object
for (URLMatcherConditionSet::Vector::iterator it = condition_sets.begin();
it != condition_sets.end(); it++) {
condition_set_id_to_event_matcher_id_.insert(
std::make_pair((*it)->id(), id));
}
id_to_event_name_[id] = event_name;
event_matchers_[event_name][id] = linked_ptr<EventMatcherEntry>(
new EventMatcherEntry(std::move(matcher) <-- correctly moved, &url_matcher_, condition_sets));
return id;
}
This is the partial definition of EventMatcher class (event_matcher.h).
On line 23
class EventMatcher {
...
std::unique_ptr<base::DictionaryValue> filter_;
...
};
The filter_ is wrapped in unique_ptr, so freed when EventMatcher object frees.
It results to use-after-free in AttachFilteredEvent on event_bindings.cc.
On line 262
std::unique_ptr<base::DictionaryValue> filter;
...
On line 277
base::DictionaryValue* filter_weak = filter.get();
int id = g_event_filter.Get().AddEventMatcher(
event_name, ParseEventMatcher(std::move(filter))); <-- Actually, ParseEventMatcher creates EventMatcher object mentioned above, with the filter_ member provided to the argument. AddEventMatcher frees filter_weak.
...
On line 283
std::string extension_id = context()->GetExtensionID();
if (AddFilter(event_name, extension_id, *filter_weak)) { <-- boom 1
bool lazy = ExtensionFrameHelper::IsContextForEventPage(context());
content::RenderThread::Get()->Send(new ExtensionHostMsg_AddFilteredListener(
extension_id, event_name, *filter_weak, lazy)); <-- boom 2
}
Actually, the Event object itself is exposed in Port object, which is returned by chrome.runtime.connect. WEB_PAGE context can call it. Like,
var Event = chrome.runtime.connect('yo').onDisconnect.constructor;
And we can provide a invalid URL matcher.
var event = new Event('app.app', [], {supportsFilters: true}); <-- Abstraction of the event binding interface.
event.addListener(function() {}, {url: [1]}); <-- Calls the AttachFilteredEvent method.
VERSION
Chrome Version: 51.0.2704.84 stable 32bit
Operating System: Windows 10 x64
REPRODUCTION CASE
I've attached the PoC as a HTML file.
,
Jul 3 2016
,
Jul 4 2016
,
Jul 17 2016
meacer: Uh oh! This issue still open and hasn't been updated in the last 14 days. This is a serious vulnerability, and we want to ensure that there's progress. Could you please leave an update with the current status and any potential blockers? If you're not the right owner for this issue, could you please remove yourself as soon as possible or help us find the right one? If the issue is fixed or you can't reproduce it, please close the bug. If you've started working on a fix, please set the status to Started. Thanks for your time! To disable nags, add the Disable-Nags label. For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
Jul 31 2016
meacer: Uh oh! This issue still open and hasn't been updated in the last 28 days. This is a serious vulnerability, and we want to ensure that there's progress. Could you please leave an update with the current status and any potential blockers? If you're not the right owner for this issue, could you please remove yourself as soon as possible or help us find the right one? If the issue is fixed or you can't reproduce it, please close the bug. If you've started working on a fix, please set the status to Started. Thanks for your time! To disable nags, add the Disable-Nags label. For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
Aug 9 2016
jinmoteam@: Thanks for the report and apologies for the late response. I'm bumping up the severity to high as this doesn't require an extension to be installed.
,
Aug 9 2016
meacer@: thanks!
,
Aug 11 2016
,
Aug 12 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/ba011d9f8322c62633a069a59c2c5525e3ff46cc commit ba011d9f8322c62633a069a59c2c5525e3ff46cc Author: meacer <meacer@chromium.org> Date: Fri Aug 12 00:23:40 2016 Ignore filtered event if an event matcher cannot be added. BUG= 625404 Review-Url: https://codereview.chromium.org/2236133002 Cr-Commit-Position: refs/heads/master@{#411472} [modify] https://crrev.com/ba011d9f8322c62633a069a59c2c5525e3ff46cc/extensions/renderer/event_bindings.cc
,
Aug 12 2016
,
Aug 12 2016
,
Aug 12 2016
,
Aug 12 2016
Your change meets the bar and is auto-approved for M53 (branch: 2785)
,
Aug 12 2016
Please merge your change by Monday (08/15) 5:00 PM PT so we can take it in for next week Beta release. Thank you.
,
Aug 15 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/7de974664bfeefa2310ee0f350437f3fdf466b91 commit 7de974664bfeefa2310ee0f350437f3fdf466b91 Author: Mustafa Acer <meacer@chromium.org> Date: Mon Aug 15 19:05:41 2016 Ignore filtered event if an event matcher cannot be added. BUG= 625404 Review-Url: https://codereview.chromium.org/2236133002 Cr-Commit-Position: refs/heads/master@{#411472} (cherry picked from commit ba011d9f8322c62633a069a59c2c5525e3ff46cc) Review URL: https://codereview.chromium.org/2250433002 . Cr-Commit-Position: refs/branch-heads/2785@{#601} Cr-Branched-From: 68623971be0cfc492a2cb0427d7f478e7b214c24-refs/heads/master@{#403382} [modify] https://crrev.com/7de974664bfeefa2310ee0f350437f3fdf466b91/extensions/renderer/event_bindings.cc
,
Aug 24 2016
,
Aug 26 2016
,
Aug 30 2016
,
Aug 30 2016
Congratulations! The panel decided to award $3,000 for this bug. A member of our finance team will be in touch shortly with next steps. *** Boilerplate reminders! *** Please do NOT publicly disclose details until a fix has been released to all our users. Early public disclosure may cancel the provisional reward. Also, please be considerate about disclosure when the bug affects a core library that may be used by other products. Please do NOT share this information with third parties who are not directly involved in fixing the bug. Doing so may cancel the provisional reward. Please be honest if you have already disclosed anything publicly or to third parties. Lastly, we understand that some of you are not interested in money. We offer the option to donate your reward to an established charity. If you prefer this option, let us know and we will also match your donation - subject to our discretion. Any rewards that are unclaimed after 12 months will be donated to a charity of our choosing. *********************************
,
Sep 1 2016
,
Sep 14 2016
,
Oct 28 2016
btw can I disclose this publicly?
,
Nov 18 2016
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
,
Apr 25 2018
|
||||||||||||||||
►
Sign in to add a comment |
||||||||||||||||
Comment 1 by jinmot...@gmail.com
, Jul 2 2016