627012 - Investigate crash reported by Adobe w/subclasses (Acrobat/Reader X) [@ _SEH_prolog ]

Status: RESOLVED FIXED

(Core Graveyard :: Plug-ins, defect)

Description

[Jim Mathies [:jimm]]

User-Agent:       Mozilla/5.0 (Windows NT 6.1; WOW64; rv:2.0b10pre) Gecko/20110117 Firefox/4.0b10pre
Build Identifier: 

detail provided by adobe:

Our Adobe Reader browser plug-in (nppdf32.dll) plays some tricks with the 
wndProcs of Firefox windows in order to resolve some issues having to do 
with embedded PDFs and function and arrow keys, onfocus/onblur messages.  We 
replace the WndProc of the Firefox windows -- saving the old one -- and 
restore it when we go away to the saved one.

There is a bug (that we can repro internally but we don't have a site handy 
outside that can do it) where, within a frame, we can get a crasher.

I've traced this down to what appears to be Firefox knowing we replace this 
function and after we un-replace it, Firefox appears to "undo" that and it's 
back to our own function again.  For the next PDF that shows, the one we 
save is our own.


Reproducible: Always

Comment 1

[Jim Mathies [:jimm]]

Rudi, can you attach a test case? A stand alone html page would be good. This would be helpful in tracking down a regression range and debugging.

Comment 2

[Rudi Sherry]

Give me a day or so to extract the essence of the crasher from our large test files.

Comment 3

[Benjamin Smedberg]

This is very likely to be caused by bug 130078: each Firefox frame no longer has a widget: only the toplevel Firefox window has a widget. Of course each Acrobat instance still has a widget, because they are windowless plugins. Are we talking about the Firefox widgets (which no longer exist in the same form) or the plugin widget?

Is there a particular reason this needs to remain security-sensitive?

Comment 4

[Jim Mathies [:jimm]]

(In reply to comment #3)
> This is very likely to be caused by bug 130078: each Firefox frame no longer
> has a widget: only the toplevel Firefox window has a widget. Of course each
> Acrobat instance still has a widget, because they are windowless plugins. Are
> we talking about the Firefox widgets (which no longer exist in the same form)
> or the plugin widget?
> 
> Is there a particular reason this needs to remain security-sensitive?

Acrobat is windowed. Looks like fall out from the crash fix for older versions in bug 531551.

Comment 5

[Rudi Sherry]

Attachment: HTML/FDF files necessary to reproduce the bug.

Unzip the attachment into a folder.
Drag main.html into Firefox.
Click the "FDF" link and wait until the PDF file (Shakespeare's sonnets) shows.
Click "Back" in Firefox.
Comment debugging the crash.

Comment 6

[Daniel Veditz [:dveditz]]

What signature(s) does this crash produce? It'd be nice to check the frequency of this crash compared to the ones fixed by bug 531551.

This is marked as a security bug, is there a vulnerability here? If so, how severe? If not we can move it to some other classification of "private" bug.

Comment 7

[Timothy Nikkel (:tnikkel)]

Widgets were removed from frames for Firefox 3.6. 130078 only removed widgets from the toplevel content document. So if this has to do with the removal of widgets from frames then it would affect Firefox 3.6 as well.

Comment 8

[Benjamin Smedberg]

Is this bug about Firefox 3.6.x or Firefox 4 betas?

Comment 9

[Rudi Sherry]

Right now this is about 3.6.x; I haven't tested 4.x yet.

Comment 10

[Jim Mathies [:jimm]]

Hmm, not having much luck reproducing. I've got Reader X installed:

    File: nppdf32.dll
    Version: 10.0.0.396
    Adobe PDF Plug-In For Firefox and Netscape 10.0.0

Trying in 3.6 and 4.0 and I'm not seeing the crash. I'll keep playing with it. Rudi, any ideas?

Comment 11

[Rudi Sherry]

I'll verify it again to make sure I sent the right files. I'm using them locally on my disk ("file" urls). Are using them locally or did you copy to a web server?

Comment 12

[Jim Mathies [:jimm]]

(In reply to comment #11)
> I'll verify it again to make sure I sent the right files. I'm using them
> locally on my disk ("file" urls). Are using them locally or did you copy to a
> web server?

Locally, would my using the 'X' version of the reader play into this?

Comment 13

[Rudi Sherry]

I'm using the 'X' version, and I see it, and am debugging it (with no progress so far).

Comment 14

[Rudi Sherry]

Progress: breakpoint set at the low-level SetWindowLongPtr() glue code in the firefox process (with condition GWL_WNDPROC) shows valid calls:

<various other calls>
I go "Back"
...nsPluginNativeWindowWin::UndoSubclassAndAssociateWindow() calls SetWindowLongPtr( wnd, GWL_WNDPROC...) with a certain proc value (0xffff0b83)  (not sure what that pointer is to, doesn't seem like it's valid).  LastError is zero both before and after, non-zero is returned.

no other calls by anyone to SetWindowLogPtr(... GWL_WNDPROC ...) until nppdf32!SubclassParent() calls it with that same window, and it returns a *different* value than passed in by UndoSubclassAndAssociateWindow() (it returns the same value I passed in which is definitely *NOT* 0xffff0b83).

I checked LastError before and after both calls: S_OK.

Is there something I'm missing?  I thought if you called it with one value, then with another, and there were no errors (and no zero-return values) it was supposed to return the first value?

Comment 15

[Rudi Sherry]

Further in-depth debugging, with test code, makes this appear to be a Windows issue (GetWindowLongPtr() not returning the value SetWindowLongPtr() set).  So far we have only tested Win 7 but have seen it on several machines.  We are going to be working with Microsoft Support on this, don't know when there will be further news.

The stack signature is the same as in bug 531551 Comment 31 (https://bugzilla.mozilla.org/show_bug.cgi?id=531551#c31).

Comment 16

[Jim Mathies [:jimm]]

I believe the odd return values (0xffff0b83) have to do with differences between using ascii vs. wide SetWindowLongPtr calls, and the mismatched ascii vs. wide calls to GetWindowLongPtr. When those match up, you'll get the real function pointer. If they don't you'll get something else.

I should have a chance to look at this today from our end.

Comment 17

[Jim Mathies [:jimm]]

Attachment: subclass trace

This is a subclass trace of this test case in 1.9.2. Things get a little confusing because we end up instantiating two instances of the plugin to handle this. One is getting torn down while the other is being brought up. 

In the end though, everything looks to be in good shape. I think what might be going wrong here is that the adobe plugin is getting confused by our resetting of the parent window subclass. Acrobat subclasses that early on, then we override it, and that subclass never seems to get reset by adobe, possibly because the window procedure has changed.

Comment 18

[Daniel Veditz [:dveditz]]

If we get more specific data about the severity of the crash please clear the sg:investigate from the whiteboard for re-triage.

Comment 19

[Jim Mathies [:jimm]]

This is a subclass crash in the container. I think we can get rid of the security sensitive rankings.

Comment 20

[Jim Mathies [:jimm]]

(In reply to comment #15)
> Further in-depth debugging, with test code, makes this appear to be a Windows
> issue (GetWindowLongPtr() not returning the value SetWindowLongPtr() set).  So
> far we have only tested Win 7 but have seen it on several machines.  We are
> going to be working with Microsoft Support on this, don't know when there will
> be further news.
> 
> The stack signature is the same as in bug 531551 Comment 31
> (https://bugzilla.mozilla.org/show_bug.cgi?id=531551#c31).

Hey Rudi,

Do you have a more current crash signature for this from 'about:crashes'? I've been searching for this in crash stats and I'm not turning anything up. I'd like to try and figure out how severe the problem is in 3.6.13.

Comment 21

[Rudi Sherry]

I don't think it's that widespread; you have to have an FDF inside a frame and go Back, both of which are not the usual mechanism.

I can't submit a crash report; the Windows "Search solution/Debug/Close" window always comes up and anything I do exits Firefox.  I've turned on "Submit crash reports" but that doesn't seem to do anything.

I duplicated this a few times just now but about:crashes only shows some old crashes from last January.  "Remove all Reports" doesn't remove them.

Is the crash-report submission completely silent?  Is there somewhere on mozilla I can look for them>

Comment 22

[Jim Mathies [:jimm]]

(In reply to comment #21)
> I don't think it's that widespread; you have to have an FDF inside a frame and
> go Back, both of which are not the usual mechanism.
> 
> I can't submit a crash report; the Windows "Search solution/Debug/Close" window
> always comes up and anything I do exits Firefox.  I've turned on "Submit crash
> reports" but that doesn't seem to do anything.
> 
> I duplicated this a few times just now but about:crashes only shows some old
> crashes from last January.  "Remove all Reports" doesn't remove them.
> 
> Is the crash-report submission completely silent?  Is there somewhere on
> mozilla I can look for them>

An installed debugger may get in the way of this. On system without that and running a release build you should just see fx close and a small crash reporter dialog displayed. about:crashes is the place to look for results. If you have crashes from old versions that are sticking around you can purge everything in:

C:\Users\(username)\AppData\Roaming\Mozilla\Firefox\Crash Reports

To get rid of them.

Comment 23

[Rudi Sherry]

OK, I've figured this out.  It is a combination of multiple pdf-like doc instances (one for the original FDF, one for the original PDF, and one for the refreshed PDF), and the fact that NPP_New for the "next" doc instance is called before NPP_Destroy for the "previous" doc instance is called.

I was fooled by the SetWindowLongA vs SetWindowLongW causing the 'proc' parameter, pointing to the same proc, to look very different depending on which was called.

This is in Reader and I'm working on a fix.

Comment 24

[Josh Aas]

(In reply to Rudi Sherry from comment #23)

> This is in Reader and I'm working on a fix.

Rudi - have you resolved this? Would be good to at least open this bug up, and even better if we can do so with a resolution.

Comment 25

[Rudi Sherry]

This was fixed in Acrobat/Reader 9.3 and the fix was carried over to Acrobat/Reader 10.0.

Note, however, that people don't always update Acrobat or Reader so we'll probably encounter an ever-decreasing tail of this :(

Comment 26

[Josh Aas]

Thanks Rudi.