ClusterFuzz is analyzing your testcase. Developers can follow the progress at https://cluster-fuzz.appspot.com/testcase?key=5758569880485888

tsepez@, do you know who might be good to take a look at this?

Dan gets to manage all of these nowdays.

Though if I'm feeling motivated, I'll sometimes just take it first.

https://codereview.chromium.org/2197793002/

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

commit 8b63f90bfb5c8cd1bc2a951e47ef65d6993e4e4f
Author: thestig <thestig@chromium.org>
Date: Tue Aug 02 04:09:00 2016

Roll PDFium 3e454bf..8f79700

https://pdfium.googlesource.com/pdfium.git/+log/3e454bf..8f79700

BUG= 62625 , 633387 , 632709 
TEST=bots
TBR=ochang@chromium.org

Review-Url: https://codereview.chromium.org/2202923002
Cr-Commit-Position: refs/heads/master@{#409134}

[modify] https://crrev.com/8b63f90bfb5c8cd1bc2a951e47ef65d6993e4e4f/DEPS

[Automated comment] DEPS changes referenced in bugdroid comments, needs manual review.

+awhalley@, is this good to take in for M53 now?

I can reproduce this bug with a slightly different address sanitizer output.

Chrome version:
---------------
1. 54.0.2822.0 (64-bit) - Tot build

==1==ERROR: AddressSanitizer: heap-use-after-free on address 0x60700001e0a0 at pc 0x5558e064ebdf bp 0x7ffc5b958590 sp 0x7ffc5b958588
READ of size 8 at 0x60700001e0a0 thread T0 (chrome)
    #0 0x5558e064ebde in GetInterForm ./out/Asan/../../third_party/pdfium/fpdfsdk/include/fsdk_baseform.h:175:52
    #1 0x5558e064ebde in UpdateFormField ./out/Asan/../../third_party/pdfium/fpdfsdk/javascript/Field.cpp:248:0
    #2 0x5558e0650006 in SetBorderStyle ./out/Asan/../../third_party/pdfium/fpdfsdk/javascript/Field.cpp:470:9
    #3 0x5558e064f8e1 in borderStyle ./out/Asan/../../third_party/pdfium/fpdfsdk/javascript/Field.cpp:393:7
    #4 0x5558e0667fb0 in JSPropSetter<Field, &Field::borderStyle> ./out/Asan/../../third_party/pdfium/fpdfsdk/javascript/JS_Define.h:116:8
....................

0x60700001e0a0 is located 32 bytes inside of 80-byte region [0x60700001e080,0x60700001e0d0)
freed by thread T0 (chrome) here:
    #0 0x5558d161a52b in operator delete(void*) ??:?
    #1 0x5558e01f24b2 in ReleaseAnnot ./out/Asan/../../third_party/pdfium/fpdfsdk/fsdk_annothandler.cpp:88:20
    #2 0x5558e01ef765 in ~CPDFSDK_PageView ./out/Asan/../../third_party/pdfium/fpdfsdk/fsdk_mgr.cpp:505:23
    #3 0x5558e01ee576 in RemovePageView ./out/Asan/../../third_party/pdfium/fpdfsdk/fsdk_mgr.cpp:363:3
    #4 0x5558d5a66142 in Unload ./out/Asan/../../pdf/pdfium/pdfium_page.cc:110:7
    #5 0x5558d5a39fea in CalculateVisiblePages ./out/Asan/../../pdf/pdfium/pdfium_engine.cc:2643:20
    #6 0x5558d5a5429d in ?? ./out/Asan/../../pdf/pdfium/pdfium_engine.cc:2287:3
    #7 0x5558d5a36be4 in Form_GetCurrentPage ./out/Asan/../../pdf/pdfium/pdfium_engine.cc:3476:21
    #8 0x5558e0666dc9 in FFI_GetCurrentPage ./out/Asan/../../third_party/pdfium/fpdfsdk/include/fsdk_mgr.h:117:14
    #9 0x5558e0666dc9 in setFocus ./out/Asan/../../third_party/pdfium/fpdfsdk/javascript/Field.cpp:3252:0
................

previously allocated by thread T0 (chrome) here:
    #0 0x5558d1619f2b in operator new(unsigned long) ??:?
    #1 0x5558e01f525c in NewAnnot ./out/Asan/../../third_party/pdfium/fpdfsdk/fsdk_annothandler.cpp:435:29
    #2 0x5558e01f2262 in NewAnnot ./out/Asan/../../third_party/pdfium/fpdfsdk/fsdk_annothandler.cpp:64:27
    #3 0x5558e01ed3c1 in LoadFXAnnots ./out/Asan/../../third_party/pdfium/fpdfsdk/fsdk_mgr.cpp:969:47
    #4 0x5558e01eca96 in GetPageView ./out/Asan/../../third_party/pdfium/fpdfsdk/fsdk_mgr.cpp:279:14
    #5 0x5558e01ba043 in FormHandleToPageView ./out/Asan/../../third_party/pdfium/fpdfsdk/fpdfformfill.cpp:47:29
    #6 0x5558e01ba043 in FORM_OnAfterLoadPage ./out/Asan/../../third_party/pdfium/fpdfsdk/fpdfformfill.cpp:644:0
    #7 0x5558d5a66391 in GetPage ./out/Asan/../../pdf/pdfium/pdfium_page.cc:125:7
    #8 0x5558e01ed85e in FFI_GetPage ./out/Asan/../../third_party/pdfium/fpdfsdk/include/fsdk_mgr.h:111:14
    #9 0x5558e01ed85e in GetPageView ./out/Asan/../../third_party/pdfium/fpdfsdk/fsdk_mgr.cpp:291:0
    #10 0x5558e01dd6b6 in GetWidget ./out/Asan/../../third_party/pdfium/fpdfsdk/fsdk_baseform.cpp:2087:28
    #11 0x5558e064ff73 in GetWidget ./out/Asan/../../third_party/pdfium/fpdfsdk/javascript/Field.cpp:298:35
    #12 0x5558e064ff73 in SetBorderStyle ./out/Asan/../../third_party/pdfium/fpdfsdk/javascript/Field.cpp:462:0
    #13 0x5558e064f8e1 in borderStyle ./out/Asan/../../third_party/pdfium/fpdfsdk/javascript/Field.cpp:393:7
...........................

tsepez,dsinclair,
This bug and  bug 630654  may need a fix in chrome pdf side rather than pdfium side.

I think both bugs happen because of this call stack.
#4 0x5558d5a66142 in Unload ./out/Asan/../../pdf/pdfium/pdfium_page.cc:110:7
    #5 0x5558d5a39fea in CalculateVisiblePages ./out/Asan/../../pdf/pdfium/pdfium_engine.cc:2643:20
    #6 0x5558d5a5429d in ?? ./out/Asan/../../pdf/pdfium/pdfium_engine.cc:2287:3
    #7 0x5558d5a36be4 in Form_GetCurrentPage ./out/Asan/../../pdf/pdfium/pdfium_engine.cc:3476:21

Is it necessary to unload invisible pages when current page is requested, even though it is necessary when PDF pages are scrolled?

Reopening as per #15 and #16. Tom, please let me know if you'd like me to take a look.

I'm going to bounce this to Dan per c17.

Is there any M53 merge is needed here? If not, please remove "Merge-Review-53" and re-request merge once #17 is addressed. 

Tom, should your change from comment #6 be merged as it sounds like #17 is a slightly different stacktrace?

Please mark security bugs as fixed as soon as the fix lands, and before requesting merges. This update is based on the merge- labels applied to this issue. Please reopen if this update was incorrect.

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

As per comments above, there is still a crash happening.

Looking at the issue it seems like, we load the PDF, and create a JS timer:

this.pageNum = 2;
function test() {
  this.getField('cmb1').borderStyle = 'dashed';
}
app.setTimeOut('test()',1000);

We then scroll down to page 2. The Chrome code releases the previous pages to save memory PDFiumEngine::CalculateVisiblePages() does the Unload. (pdfium_engine.cc:2643). This cleans up the page that has the form field 'cmb1' on it. The timer then fires, we try to access the widget and get the use-after-free error.

I'm not sure the right fix for this at the moment. We could probably fix things so we don't access the right widget, but it seems like the widget should exist in order to set the border style on it if it comes back into focus again.

dsinclair, PDF file has below mentioned Javascript too.
Format method of cmb1 Combo Box:
this.getField('txt1').setFocus();

Is there any M53 merge is needed here? If not, please remove "Merge-Review-53".
awhalley@, could you ptal?

Removing the merge labels as there is still work to do around these files to make them work properly.

Got my BUG= wrong. https://codereview.chromium.org/2251453002/ contains a fix for testfocus2.pdf so it works without ASAN issues for me.

chamal.desilva@ can you please test it out once it lands in canary to see if it solves your issues as well?

Sure. I'll test once the fix lands.

Rolled DEPS in r412142 to pick up https://pdfium.googlesource.com/pdfium.git/+/ef523dd36aea991084b8b934df846014a5c09c6f in Chromium on trunk. Not sure why bugdroid is being slow.

dsinclair, Bug fixed in chrome version 54.0.2831.0.

dsinclair, I am sorry for being bit annoying.
It is possible to reproduce same bug with a modified test case.

Chrome version - 54.0.2831.0
Steps:
1. Open attached 632709_2.pdf with chrome built with Address Sanitizer.
2. Wait 6 seconds.

Modified JavaScript in Document JavaScript of PDF file -
function movePage() {
this.pageNum = 2;
}

function test() {
this.getField('cmb1').value = 'two';
this.getField('cmb1').borderStyle = 'dashed';
}

app.setTimeOut('movePage()',3000);
app.setTimeOut('test()',6000);

* Time outs are added only for the purpose of making sure everything happens in correct order.

Deleted: 632709_2.pdf 3.6 KB

632709_2.pdf 3.6 KB Download

Thanks for the updated test case, I'll take a look. This bug just doesn't want to be fixed.

dsinclair: 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

The following revision refers to this bug:
  https://pdfium.googlesource.com/pdfium.git/+/1df1efa3921841fb5fc7fc15e8112eed4375de9f

commit 1df1efa3921841fb5fc7fc15e8112eed4375de9f
Author: dsinclair <dsinclair@chromium.org>
Date: Wed Sep 07 16:55:37 2016

Fixup CPDFSDK_PageView and CPDF_Page interactions.

There are several issues when CPDFSDK_PageView and CPDF_Page interact,
especially around deletion. This Cl fixes up several places where things
go wrong working with these objects.

BUG= chromium:632709 

Review-Url: https://codereview.chromium.org/2319663002

[modify] https://crrev.com/1df1efa3921841fb5fc7fc15e8112eed4375de9f/fpdfsdk/fpdfview.cpp
[modify] https://crrev.com/1df1efa3921841fb5fc7fc15e8112eed4375de9f/fpdfsdk/fsdk_mgr.cpp
[modify] https://crrev.com/1df1efa3921841fb5fc7fc15e8112eed4375de9f/fpdfsdk/include/fsdk_mgr.h
[modify] https://crrev.com/1df1efa3921841fb5fc7fc15e8112eed4375de9f/fpdfsdk/javascript/Field.cpp

This should fix the pdf in comment #33. If there are other issues found, please file new bugs.

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

commit 8400dbfc0a44d96ab007313f4c4aeac3d32c8bf9
Author: pdfium-deps-roller <pdfium-deps-roller@chromium.org>
Date: Wed Sep 07 18:39:29 2016

Roll src/third_party/pdfium/ cb92dc762..1df1efa39 (2 commits).

https://pdfium.googlesource.com/pdfium.git/+log/cb92dc762d5c..1df1efa39218

$ git log cb92dc762..1df1efa39 --date=short --no-merges --format='%ad %ae %s'
2016-09-07 dsinclair Fixup CPDFSDK_PageView and CPDF_Page interactions.
2016-09-07 tsepez Maybe avoid a copy in CPDF_RenderStatus::SelectClipPath

BUG= 632709 

TBR=dsinclair@chromium.org

Review-Url: https://codereview.chromium.org/2323443002
Cr-Commit-Position: refs/heads/master@{#417007}

[modify] https://crrev.com/8400dbfc0a44d96ab007313f4c4aeac3d32c8bf9/DEPS

awhalley, Thanks a lot for forwarding this bug to reward panel. But I don't want a reward for this bug since I got rewarded for 630654. Both bugs have the same cause.

Thank you for mentioning it and saving the panel's time to spot that :-)

[Automated comment] DEPS changes referenced in bugdroid comments, needs manual review.

This change meets the bar and is approved for merging into M54.

Please be aware that crbug.com/645122 and crbug.com/642307 are also updating the DEPS for pdfium, and were approved recently.

Is that branch 2840?

The following revision refers to this bug:
  https://chrome-internal.googlesource.com/chrome/tools/buildspec/+/f68ef3151b196b3bd53f96ad94f89e02c90d93df

commit f68ef3151b196b3bd53f96ad94f89e02c90d93df
Author: Dan Sinclair <dsinclair@chromium.org>
Date: Tue Sep 13 19:19:10 2016

[Automated comment] DEPS changes referenced in bugdroid comments, needs manual review.

Approving merge to M53 branch 2785 as it is requested by awhalley@ at #52 assuming it is good to take for M53. Please merge ASAP. Thank you.

Please hold off M53 merge for now. kerz@ or I will update the bug if merge is needed. Thank you.

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

We decided NOT to take this merge in for M53 respin this week. 

The following revision refers to this bug:
  https://pdfium.googlesource.com/pdfium.git/+/c5267c54ea32ade03bf10e6bb203e34e3fcbd77d

commit c5267c54ea32ade03bf10e6bb203e34e3fcbd77d
Author: dsinclair <dsinclair@chromium.org>
Date: Fri Nov 04 22:35:12 2016

Revert: Only create widgets if really needed

This CL reverts ef523dd36aea991084b8b934df846014a5c09c6f which causes issues
with syncing of form fields over pages.

The initial bug had follow on fixes which seem to have rendered this fix
un-needed.

BUG= chromium:632709 ,  chromium:661294 

Review-Url: https://codereview.chromium.org/2473103003

[modify] https://crrev.com/c5267c54ea32ade03bf10e6bb203e34e3fcbd77d/fpdfsdk/cpdfsdk_interform.cpp
[modify] https://crrev.com/c5267c54ea32ade03bf10e6bb203e34e3fcbd77d/fpdfsdk/cpdfsdk_interform.h
[modify] https://crrev.com/c5267c54ea32ade03bf10e6bb203e34e3fcbd77d/fpdfsdk/javascript/Field.cpp
[modify] https://crrev.com/c5267c54ea32ade03bf10e6bb203e34e3fcbd77d/fpdfsdk/javascript/Field.h

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

commit 4093035dccdddf3a51f4035dfe9d3c90809ffb94
Author: pdfium-deps-roller <pdfium-deps-roller@chromium.org>
Date: Sat Nov 05 00:33:18 2016

Roll src/third_party/pdfium/ ec64cee9a..04bebfe59 (4 commits).

https://pdfium.googlesource.com/pdfium.git/+log/ec64cee9accc..04bebfe590d6

$ git log ec64cee9a..04bebfe59 --date=short --no-merges --format='%ad %ae %s'
2016-11-04 thestig Implement FPDF_VIEWERREF_GetName() API.
2016-11-04 tsepez Pass object to ReplaceIndirectObjectIfHigherGeneration() by unique_ptr.
2016-11-04 tsepez CHECK for re-addition of indirect objects
2016-11-04 dsinclair Revert: Only create widgets if really needed

BUG= 632709 , 661294 

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/2480923002
Cr-Commit-Position: refs/heads/master@{#430089}

[modify] https://crrev.com/4093035dccdddf3a51f4035dfe9d3c90809ffb94/DEPS

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