Issue metadata
Sign in to add a comment
|
Security: potential UAF in pdfium timer
Reported by
jinmot...@gmail.com,
Jan 10 2017
|
||||||||||||||||||||||
Issue description
VULNERABILITY DETAILS
There is js timer implementation in src/third_party/pdfium/fpdfsdk/javascript/app.cpp.
When GlobalTimer is removed, it must delete <timer ID>-><GlobalTimer> mapping, but if m_TimerID is 0, it doesn't remove it.
On Line 81:
GlobalTimer::~GlobalTimer() {
if (!m_nTimerID)
return;
if (GetRuntime())
m_pFormFillEnv->GetSysHandler()->KillTimer(m_nTimerID);
GetGlobalTimerMap()->erase(m_nTimerID);
}
Additionally, GetGlobalTimerMap is referenced in GlobalTimer::Trigger, which is called as timer callback. However if m_nTimerID is 0, it doesn't kill the timer, so it seems like UAF.
VERSION
Chrome Version: 55.0.2883.87 stable
Operating System: Windows 10
REPRODUCTION CASE
For now there is memory leak on app.setTimeOut / app.clearTimeOut, so can't reproduce in my laptop.
,
Jan 10 2017
Possible mitigations would be adjusting timerid as 64bit, or just remove the check.
,
Jan 11 2017
Thanks for the report. Having an ID that can't overflow seems desirable, even if the check is removed.
,
Jan 11 2017
,
Jan 11 2017
,
Jan 11 2017
https://cs.chromium.org/chromium/src/third_party/pdfium/public/fpdf_formfill.h?rcl=1484151649&l=508 means that the timer ID comes from the embedder as an int and can't be enlarged without a public API change. So much for that idea.
,
Jan 11 2017
,
Jan 12 2017
,
Jan 12 2017
,
Jan 12 2017
,
Jan 12 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/34ff66f6a7ed64c19c9494b0327a7a4037b7b2ff commit 34ff66f6a7ed64c19c9494b0327a7a4037b7b2ff Author: pdfium-deps-roller <pdfium-deps-roller@chromium.org> Date: Thu Jan 12 22:28:15 2017 Roll src/third_party/pdfium/ db7647083..98d00b230 (4 commits). https://pdfium.googlesource.com/pdfium.git/+log/db7647083d0a..98d00b230aa1 $ git log db7647083..98d00b230 --date=short --no-merges --format='%ad %ae %s' 2017-01-12 dsinclair Remove used items from the CSS code. 2017-01-12 tsepez Don't put timers with ID == 0 into the global timer map. 2017-01-12 tsepez Custom toString() methods may delete annots. 2017-01-12 npm Fix leak in OJPEGReadHeaderInfoSecTablesAcTable when read fails. BUG= 679649 , 679643 , 680520 Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+/master/autoroll/README.md If the roll is causing failures, see: http://www.chromium.org/developers/tree-sheriffs/sheriff-details-chromium#TOC-Failures-due-to-DEPS-rolls TBR=dsinclair@chromium.org Review-Url: https://codereview.chromium.org/2627073004 Cr-Commit-Position: refs/heads/master@{#443386} [modify] https://crrev.com/34ff66f6a7ed64c19c9494b0327a7a4037b7b2ff/DEPS
,
Jan 17 2017
,
Jan 17 2017
,
Jan 18 2017
,
Jan 27 2017
,
Jan 27 2017
The panel decided to award $500 for this bug. Cheers!
,
Jan 27 2017
,
Feb 3 2017
,
Feb 3 2017
This bug requires manual review: DEPS changes referenced in bugdroid comments. Please contact the milestone owner if you have questions. Owners: amineer@(clank), cmasso@(bling), ketakid@(cros), govind@(desktop) For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
Feb 3 2017
+awhalley@ for M57 merge review.
,
Feb 12 2017
+govind: good to approve for 57. Note that we only need a merge of "Don't put timers with ID == 0 into the global timer map." not necessarily the full DEPS roll.
,
Feb 13 2017
Approving merge to M57 branch 2987. Please refer to comment #21 before merging. Thank you.
,
Feb 14 2017
If possible, please merge your change to M57 branch 2987 today (Tuesday) before 5:00 PM PT so we can pick it up for this week beta release. Thank you.
,
Feb 16 2017
This issue has been approved for a merge. Please merge the fix to any appropriate branches as soon as possible! If all merges have been completed, please remove any remaining Merge-Approved labels from this issue. 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
,
Feb 16 2017
Please merge your change to M57 branch 2987 before 5:00 PM PT Friday (02/17), so we can pick it up for next week Beta release. Thank you.
,
Feb 17 2017
Please merge your change to M57 branch 2987 before 5:00 PM PT Monday (02/20), so we can pick it up for next week Beta release. Thank you.
,
Feb 20 2017
This issue has been approved for a merge. Please merge the fix to any appropriate branches as soon as possible! If all merges have been completed, please remove any remaining Merge-Approved labels from this issue. 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
,
Feb 21 2017
Please merge your change to M57 branch 2987 by 5:00 PM PT Tuesday (02/21) so we can pick it up for this week beta release. Thank you.
,
Feb 25 2017
Please merge your change to M57 branch 2987 by 5:00 PM PT Monday (02/27) so we can take it in for next week last M57 Desktop Beta release before Stable promotion. Thank you.
,
Feb 25 2017
sheriffbot is being a bit silly. The fix made it in before M57 branched. We should have merged to M56, but now it's probably too late for that, so nothing to do here except for removing the merge label.
,
Mar 6 2017
,
Mar 6 2017
,
Mar 8 2017
,
Apr 26 2017
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
, Jan 10 2017