Closed Bug 1333486 Opened 3 years ago Closed 3 years ago

Crash in @0x0 | idmcchandler7_64.dll@0x238bf

Categories

(Toolkit :: Blocklist Policy Requests, defect, critical)

x86_64
Windows 10
defect
Not set
critical

Tracking

()

RESOLVED FIXED
mozilla55
Tracking Status
firefox52 --- unaffected
firefox-esr52 --- unaffected
firefox53 + wontfix
firefox54 + fixed
firefox55 --- fixed

People

(Reporter: marcia, Assigned: philipp)

References

Details

(Keywords: crash)

Crash Data

Attachments

(1 file, 5 obsolete files)

This bug was filed from the Socorro interface and is 
report bp-0f9ddf1d-1b8a-430a-8a04-a16632170124.
=============================================================

Seen while looking at crash stats. This crash appears on various branches (Nightly, Aurora, etc) and is corrected to the IDM addons: http://bit.ly/2jaJlr6
hi, could you take a look here?
Flags: needinfo?(charles)
Crash Signature: [@ @0x0 | idmcchandler7_64.dll@0x238bf] [@ @0x0 | idmcchandler7_64.dll@0x1feaf] → [@ @0x0 | idmcchandler7_64.dll@0x238bf] [@ @0x0 | idmcchandler7_64.dll@0x1feaf] [@ @0x0 | idmcchandler7_64.dll@0x238bf] [@ @0x0 | idmcchandler7_64.dll@0x233af] [@ @0x0 | idmcchandler7_64.dll@0x233cf] [@ @0x0 | idmcchandler7_64.dll@0x22e6f] [@ @0x0 |…
We don't support these versions of Firefox. Our extension only loads in predefined Firefox versions which are explicitly listed in the extension itself. This crash may persists for users who turned off compatibility checking in Firefox.
I should give more information today

We have the following line in install.rdf
<em:strictCompatibility>true</em:strictCompatibility>

Also we have the following line for Firefox
<em:maxVersion>52.*</em:maxVersion>

We tried to disable the verification of digital signatures in extensions in Firefox 54 (using about:config) with the following line

xpinstall.signatures.required = false

Then we changed the maxVersion to 54.*, and we could repeat the crash. It happens because our DLL uses XPCOM, but you removed it from Aurora 53. We have made an additional check to find out whether XPCOM is supported. Once customers who use incompatible extension update IDM, there will be no crash now

We knew that you were going to get rid of XPCOM sooner or later. We are working on the transition of our extension to webextension right away.
The crash is happening to users that are forcibly enabling the addon despite its incompatibility, so I think we can't do anything about it.

Charles, thanks for the information.
Status: NEW → RESOLVED
Closed: 3 years ago
Flags: needinfo?(charles)
Resolution: --- → WONTFIX
Actually you can do the following to stop these crashes. You could just return NS_GetServiceManager export function to xul.dll, which would be empty, but all its calls should return an error, for example, NS_ERROR_NOT_IMPLEMENTED. After that all IDM old versions should stop crashing Firefox
(In reply to Charles Jones from comment #5)
> Actually you can do the following to stop these crashes. You could just
> return NS_GetServiceManager export function to xul.dll, which would be
> empty, but all its calls should return an error, for example,
> NS_ERROR_NOT_IMPLEMENTED. After that all IDM old versions should stop
> crashing Firefox

I don't think the volume of the crash is large enough to warrant this, given that most people will sooner or later update IDM.

CCing bsmedberg in case he wants to do it anyway.

Another solution would be to blocklist the old IDM versions for Firefox >= 53.
No, we aren't going to implement NS_GetServiceManager. It would very likely cause other software to crash.

I do think we should blocklist the old IDM versions for FF53+. What exact versions numbers does that affect?
Status: RESOLVED → REOPENED
Component: Other → Blocklisting
Flags: needinfo?(mcastelluccio)
Product: External Software Affecting Firefox → Toolkit
Resolution: WONTFIX → ---
Redirecting needinfo to Charles. Which versions should be block for Firefox >= 53? (The extension will be automatically reenabled when the user updates IDM).

Will the blocklisting work for users who have forcibly enabled the extension? According to Charles, the extension already has <em:maxVersion>52.*</em:maxVersion>.
Flags: needinfo?(mcastelluccio) → needinfo?(charles)
The maxVersion doesn't have any effect unless the add-on has strictCompatibility set in the manifest. Even if that were the case, the blocklist takes precedence over compatibility.
We have a bad experience with your last black listing when it was done incorrectly. We worry that you may mix something, and our users may suffer again. Note that we have strictCompatibility set to true in install.rdf, look at comment # 3. Please note comment # 4, where you decided to do nothing about it, because users are using some hacking ways to forcibly load our extension. If they can modify compatibility, they can modify the extension version number as well, and your blocklisting will not work.

Note that the number of crashes should decrease quickly because millions of customers update IDM every day. If you decide to blacklist anyway, please triple check everything. You need to blacklist up to 6.26.11 inclusive for Firefox 53 and above. Please do not blacklist anything for Firefox versions below 53. Please do not blacklist extensions above 6.26.11
(In reply to Jorge Villalobos [:jorgev] from comment #9)
> The maxVersion doesn't have any effect unless the add-on has
> strictCompatibility set in the manifest. Even if that were the case, the
> blocklist takes precedence over compatibility.

Jorge, the addon has strictCompatibility set in the manifest. So it looks like the users who are crashing are forcibly enabling the extension.
Does the user's choice have precedence over blacklisting, or is it the other way around? If blocklisting has precedence, then I suppose we can blocklist (following what Charles said at the end of comment 10).
Flags: needinfo?(charles) → needinfo?(jorge)
There are ways to disable compatibility checks in the preferences, just like there are ways to disable the blocklist as well. Determined users can work around any blocks we put, but they should be a very small group.

If the volume for this crash is not very small, I suggest we move forward with the block as specified in comment #10.
Flags: needinfo?(jorge)
Depends on: 1338832
Crash Signature: | idmcchandler7_64.dll@0x22e7f] → | idmcchandler7_64.dll@0x22e7f] [@ @0x0 | idmcchandler7_64.dll@0x2343f] [@ @0x0 | idmcchandler5_64.dll@0x1f0ea]
Agree about blocklisting. Can you make that happen?
Assignee: nobody → jorge
Hardware: Unspecified → x86_64
The addon was blocklisted, but we're still seeing crashes (most of them in 53 Beta).

Most crashes have 6.23.19, 6.23.22 and 6.27.3 (which are outside the blocking range suggested by Charles).

Charles, I assume we should block these versions too for Firefox >= 53?
Flags: needinfo?(charles)
The block in bug 1338832 covers versions 0 - 6.26.11, so only 6.27.3 is out of the current block range. Note that in bug 1338170 we discovered that overriding compatibility checks also overrides a softblock, so the block was changed to a hardblock yesterday.
(In reply to Jorge Villalobos [:jorgev] from comment #15)
> The block in bug 1338832 covers versions 0 - 6.26.11, so only 6.27.3 is out
> of the current block range. Note that in bug 1338170 we discovered that
> overriding compatibility checks also overrides a softblock, so the block was
> changed to a hardblock yesterday.

Yes, sorry, I confused the version numbers :)

Charles, should we block 6.27.3 too for Firefox >= 53?
Duplicate of this bug: 1338170
[Tracking Requested - why for this release]: Top crasher in 53 Beta.
Crash Signature: | idmcchandler7_64.dll@0x22e7f] [@ @0x0 | idmcchandler7_64.dll@0x2343f] [@ @0x0 | idmcchandler5_64.dll@0x1f0ea] → | idmcchandler7_64.dll@0x22e7f] [@ @0x0 | idmcchandler7_64.dll@0x2343f] [@ @0x0 | idmcchandler5_64.dll@0x1f0ea] [@ @0x0 | ffi_call]
We don't have official version to be run in Firefox 53. It looks like customers modify compatibility, e.g. modify install.rdf and about:config settings of Firefox. Also 6.27.3 should not crash Firefox, maybe customers install XPI only, and use old versions of binary DLLs. Please note that all these customers use unofficial (cracked, patched) versions of IDM. Jorge replied that you had changed softblock to hardblock. Will it help to stop crashes with such modified compatibility? Our main concern is that you must not blocklist 6.27.3 for Firefox < 53.
(In reply to Charles Jones from comment #19)
> We don't have official version to be run in Firefox 53. It looks like
> customers modify compatibility, e.g. modify install.rdf and about:config
> settings of Firefox. Also 6.27.3 should not crash Firefox, maybe customers
> install XPI only, and use old versions of binary DLLs. Please note that all
> these customers use unofficial (cracked, patched) versions of IDM. Jorge
> replied that you had changed softblock to hardblock. Will it help to stop
> crashes with such modified compatibility? Our main concern is that you must
> not blocklist 6.27.3 for Firefox < 53.

Thanks for the info. We will only blocklist it for Firefox >= 53.
Depends on: 1347610
Please do not blocklist anything for Firefox < 53. Also since cracked versions modify compatibility, they may modify the version number of the extension. May you block the extensions with damaged digital signature?
(In reply to Charles Jones from comment #22)
> Please do not blocklist anything for Firefox < 53. Also since cracked
> versions modify compatibility, they may modify the version number of the
> extension. May you block the extensions with damaged digital signature?

Yes, I've opened bug 1347610 to only block it for >= 53.

In order to avoid any problems in the future, could you bump the major version number from 6 to 7 when you have an extension compatible with Firefox 53? This way we could blocklist all 6.* for Firefox >= 53.

I thought extensions with an invalid digital signature would always be automatically blocked. Isn't it the case, Jorge?
Flags: needinfo?(jorge)
Regarding blocking 6.27.3, I misinterpreted. It is actually compatible with Firefox 53, so we should not block it.
(In reply to Marco Castelluccio [:marco] from comment #23)
> I thought extensions with an invalid digital signature would always be
> automatically blocked. Isn't it the case, Jorge?

Yes, Firefox Release and Beta don't allow unsigned add-ons to be loaded. There are some exceptions, but they don't apply in this case.
Flags: needinfo?(jorge)
I have 6.27.3 version of IDM add-on, and the correct version of idmcchandler7_64.dll is 6.27.5.1 while you have 6.26.9.1. in your crash report. It confirms that people manage to use new install.rdf (xpi) with old binary DLLs, thus there is no sense to block by add-on version number. Can you disable specific DLL versions for Firefox >=53? How many such crashes do you have? Maybe we can ignore them?
(In reply to Charles Jones from comment #26)
> I have 6.27.3 version of IDM add-on, and the correct version of
> idmcchandler7_64.dll is 6.27.5.1 while you have 6.26.9.1. in your crash
> report. It confirms that people manage to use new install.rdf (xpi) with old
> binary DLLs, thus there is no sense to block by add-on version number. Can
> you disable specific DLL versions for Firefox >=53? How many such crashes do
> you have? Maybe we can ignore them?

We can try to block the old IDM DLLs; how does the addon react if it can't load the DLL?

Yes, they are a limited number and we could probably ignore them.
If add-on cannot load DLL, add-on just won't work, and it should look like it's completely disabled.

It should be a good solution to disable specific DLL and its specific versions for Firefox >=53 if you can do it correctly.
which dll files and versions should we target for blocklisting in 53+ then?

i see the following showing up in correlation data for these crashes:
idmcchandler7.dll (up to version 6.27.5.1)
idmcchandler7_64.dll (up to version 6.27.5.1)
idmcchandler5.dll (up to version 6.23.15.1)
Do these DLLs appear in some crashes? Or the crash is inside of these DLLs? 

If these DLLs appear in some crashes, it may be due to IDM popularity.

Please note that we fixed this particular crash in all recent versions of idmcchandler7_64.dll. Also the crash did not happen for 32 bit version of idmcchandler7.dll
Flags: needinfo?(charles)
no, those were just the dll correlations for some of the crash signatures associated to this bug - [@ @0x0 | ffi_call ] in particular...
Attached patch bug1333486.patch (obsolete) — Splinter Review
ok, i did take another look at just the [@ @0x0 | ffi_call ] signature crashes remaining on firefox 53.0b4 which has the addon hardblock entry baked in already - idmcchandler7.dll 6.27.2.1 was the most recent crashing version i saw.
the proposed patch would attempt to block this version of the dll (and everything below) and all versions of idmcchandler5.dll from being loaded into firefox.

does this look right to you - is anything missing or does the block go too far?
Attachment #8849745 - Flags: feedback?(charles)
May you send a link or links to crash reports? We will investigate it.
https://crash-stats.mozilla.com/report/index/ebd215f3-0c31-41c4-8aaf-0306e2170321#tab-details is one example, but i can only go by the public crash data. i don't know your version scheme and since when the fix was supposed to be included, so please just tell if the blocking range should be adjusted.
I found no evidence that the crash happened because of our DLL. I can confirm that we have fixed the crash in 6.27.3.1. Thus, you should block <=6.27.2.1 for Firefox >=53. The names of our 2 DLLs to block are idmcchandler7_64 and idmcchandler7.
Attached patch bug1333486v2.patch (obsolete) — Splinter Review
hi marco, could you review the dll blocklisting entry? thanks
Attachment #8849745 - Attachment is obsolete: true
Attachment #8849745 - Flags: feedback?(charles)
Attachment #8850168 - Flags: review?(mcastelluccio)
Attached patch bug1333486v2.patch (obsolete) — Splinter Review
prior attachment lacked a proper commit message. also marco seems to be on pto at the moment as a reviewer...
Attachment #8850168 - Attachment is obsolete: true
Attachment #8850168 - Flags: review?(mcastelluccio)
Attachment #8851017 - Flags: review?(benjamin)
BTW do you have any ideas how our old extension could be loaded in Firefox 53?

I see that you added a hard block. If we change Firefox version in install.rdf, and do not sign it, it does not load because the signature is invalid.

Maybe there are some add-ons which are altered by some 3rd party (not us), and they sign them on addons.mozilla.org? May you take a look in your database of signed add-ons?
(In reply to Charles Jones from comment #38)
> Maybe there are some add-ons which are altered by some 3rd party (not us),
> and they sign them on addons.mozilla.org? May you take a look in your
> database of signed add-ons?

Good call. I see a couple of forks without strict compatibility, which could be the cause of some crashes. I don't have any data on their usage, so they could just be personal use forks or upload tests. I'll add them to the block, either way.
Comment on attachment 8851017 [details] [diff] [review]
bug1333486v2.patch

Given our new policy that external DLLs are not be allowed in Firefox beginning with Firefox 53, this should be an ALL_VERSIONS block. r=me with that or please come back explaining why.
Attachment #8851017 - Flags: review?(benjamin) → review+
charles, please see comment #40. are you aware of this policy change & what kind of time would you need to prepare before we'd enforce this (& blocklist all versions of the dlls)?
https://blog.mozilla.org/addons/2017/01/24/preventing-add-ons-third-party-software-from-loading-dlls-into-firefox/
Flags: needinfo?(charles)
We don't fully understand the syntax of your blocklist attachment, but first, please, do not blocklist anything for Firefox < 53

BTW Do you have any crashes right away after implementing the block list and hard block?

Regarding Comment 41, we were forced to speed up the development of webextension add-on because of that. And we have already included the test version of IDM webextension add-on for Firefox >=53 in our current distribution files, and it will work by default in Firefox 53 instead of our old SDK version with all new and old IDM installations

But this extension does not have the full functionality of old SDK version. In particular we need the feature below

https://bugzilla.mozilla.org/show_bug.cgi?id=1255894

If you could speed up developing the feature above, it would be helpful.

Also because we did not test the new extension well for millions of users, it would be better to have a backup SDK version for Firefox 53 for those users who may face some rare compatibility problems with web extension. Thus, you may disable our DLLs completely in Firefox 54. It should give us enough time for smooth transition and for possible bug fixes.
Flags: needinfo?(charles)
(In reply to Charles Jones from comment #42)
> We don't fully understand the syntax of your blocklist attachment, but
> first, please, do not blocklist anything for Firefox < 53

this DLL blocklist is works differently to the addons blocklist that is downloaded by firefox once a day. the dll blocklist is baked into the firefox executable, so the changeset would need to be uplifted to the release channels where it should a apply & a new firefox version would have to be released then (we certainly wouldn't do that for anything <53).

> BTW Do you have any crashes right away after implementing the block list and
> hard block?

53.0b4 and later should have had the addon hardblock out of the box - so all the crashes thereafter should have happend in these circumstances: https://crash-stats.mozilla.com/signature/?build_id=%3E%3D20170316212821&signature=%400x0%20%7C%20ffi_call#reports

> Regarding Comment 41, we were forced to speed up the development of
> webextension add-on because of that. And we have already included the test
> version of IDM webextension add-on for Firefox >=53 in our current
> distribution files, and it will work by default in Firefox 53 instead of our
> old SDK version with all new and old IDM installations
> 
> But this extension does not have the full functionality of old SDK version.
> In particular we need the feature below
> 
> https://bugzilla.mozilla.org/show_bug.cgi?id=1255894
> 
> If you could speed up developing the feature above, it would be helpful.

since i'm only a volunteer myself i can't speak to that. maybe someone from teh addons team reading a long here can comment.

> Also because we did not test the new extension well for millions of users,
> it would be better to have a backup SDK version for Firefox 53 for those
> users who may face some rare compatibility problems with web extension.
> Thus, you may disable our DLLs completely in Firefox 54. It should give us
> enough time for smooth transition and for possible bug fixes.

thanks for the info!
(In reply to Charles Jones from comment #42)
> https://bugzilla.mozilla.org/show_bug.cgi?id=1255894
> 
> If you could speed up developing the feature above, it would be helpful.

The bug is active and there's a patch awaiting review, so I don't think there's much more we can do about it. It might be worth requesting an uplift once it lands, but that depends on how complex the fix is.
Attached patch bug1333486v3.patch (obsolete) — Splinter Review
carrying over the review from comment #40 ...
Attachment #8851017 - Attachment is obsolete: true
Attachment #8853094 - Flags: review+
how should we proceed here in regard to charles' concerns in comment #41 about the quality of the extension if we block all dll versions in a short timeframe?

would landing and uplifting the ALL_VERSIONS block to 54, but sticking to attachment 8851017 [details] [diff] [review] (targeting the known bad versions only) for 53 be an acceptable solutions in your eyes?
Flags: needinfo?(benjamin)
We support comment 46, It's better for us if you block idmcchandler7_64 and idmcchandler7 <=6.27.2.1 for Firefox =53, and ALL_VERSIONS for Firefox >(=)54. Note that we found no crashes in your crash reports with idmcchandler7_64 and idmcchandler7 =>6.27.3. And once again, I can confirm that we have fixed the crash in 6.27.3.1.
philipp, I am not the product decision-maker about blocklisting; that is Kev. However my strong recommendation here is to block in 53+ per our blocklisting policy.
Flags: needinfo?(benjamin)
hi kev, do you concur with that route of action? (see discussion from comment #41 onwards)
Flags: needinfo?(kev)
Keywords: checkin-needed
Pushed by ryanvm@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/1ef73669e8fc
Blocklist Internet Download Manager .dll crashing on Firefox 53+. r=bsmedberg
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/1ef73669e8fc
Status: REOPENED → RESOLVED
Closed: 3 years ago3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
Please request Aurora/Beta approval on this when you get a chance.
Flags: needinfo?(jorge)
Assignee: jorge → madperson
Flags: needinfo?(jorge) → needinfo?(madperson)
i think i can go ahead with aurora - for beta i'm waiting for confirmation from kev which route to take (ALL_VERSIONS block or only the known bad versions).
Flags: needinfo?(madperson)
Comment on attachment 8853094 [details] [diff] [review]
bug1333486v3.patch

Approval Request Comment
[Feature/Bug causing the regression]: external program hooking into firefox
[User impact if declined]: crashes
[Is this code covered by automated tests?]: n/a
[Has the fix been verified in Nightly?]: no, too little crashing volume on nightly/aurora channels. this would need to go all the way to beta to see an effect.
[Needs manual test from QE? If yes, steps to reproduce]: n/a
[List of other uplifts needed for the feature/fix]: n/a
[Is the change risky?]: no
[Why is the change risky/not risky?]: the patch is making use of the purpose-built dll blocklisting mechanism and just adding a new dll entry there
[String changes made/needed]: n/a
Attachment #8853094 - Flags: approval-mozilla-aurora?
Comment on attachment 8853094 [details] [diff] [review]
bug1333486v3.patch

Block dlls to avoid the crash. Aurora54+.
Attachment #8853094 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
You discuss actively the blocklist for Firefox 54+. But what was your final decision about blocking our DLLs in Firefox 53? You already had a correct attachment in comment 36 where only old versions of our DLLs supposed to be blocked. Note that only these old versions caused the crash. It must be done in Firefox 53. Have you already implemented and released this DLL block list?
nothing landed in regards to 53 yet - i'm still waiting for a reply from kev needham to comment #49 about that decision...
Howdy folks,

Per comment #42 (and after reviewing the entirety of this bug), the DLL loaded by the extension hasn't been adequately tested by the developer, and we're seeing crash volumes attributed to the DLL. Those crashes negatively affect the user experience, and until we're satisfied that said DLL won't crash Firefox, a blocklisting is warranted.

From what I understand, we're seeing high levels of crashes in 53. 

r=me for DLL blocklisting in 53 until we're satisfied the problem has been addressed and doesn't impact Firefox and its users. 



(In reply to Charles Jones from comment #42)
> Also because we did not test the new extension well for millions of users,
> it would be better to have a backup SDK version for Firefox 53 for those
> users who may face some rare compatibility problems with web extension.
> Thus, you may disable our DLLs completely in Firefox 54. It should give us
> enough time for smooth transition and for possible bug fixes.
Flags: needinfo?(kev)
# 9 top crash on beta right now, so I'd like to blocklist this in 53.  I don't want to try to cherry pick specific versions this late in the release cycle, for the in-tree blocklist.
Attached patch bug1333486v3-beta.patch (obsolete) — Splinter Review
Approval Request Comment
[Feature/Bug causing the regression]: external program hooking into firefox
[User impact if declined]: crashes
[Is this code covered by automated tests?]: n/a
[Has the fix been verified in Nightly?]: no, too little crashing volume on
nightly/aurora channels. this would need to go all the way to beta to see an
effect.
[Needs manual test from QE? If yes, steps to reproduce]: n/a
[List of other uplifts needed for the feature/fix]: n/a
[Is the change risky?]: no
[Why is the change risky/not risky?]: the patch is making use of the
purpose-built dll blocklisting mechanism and just adding a new dll entry there
[String changes made/needed]: n/a
Attachment #8855046 - Flags: approval-mozilla-beta?
Kev, it looks like you did not read carefully

We did not test well new extension based on webextension. This extension
is without DLL.

I can confirm that we have fixed the crash in 6.27.3.1 and newer
versions of DLL. Note that all new versions of our old add-on with new
DLLs are already installed on thousands(millions?) of computers, and we
have seen no crashes with new versions of DLL

We also asked if you saw any crashes with new DLLs, and nobody replied about
them either

I don't understand what is the reason to block new versions of DLLs if
you have no crashes for them and they perform good

We agreed to block ALL version in Firefox 54 and above, but need some
transition time to test new add-on based on webextension, and would
appreciate having old add-on for some time as a backup solution 

(In reply to Kev Needham [:kev] from comment #59)
> Howdy folks,
> 
> Per comment #42 (and after reviewing the entirety of this bug), the DLL
> loaded by the extension hasn't been adequately tested by the developer, and
> we're seeing crash volumes attributed to the DLL. Those crashes negatively
> affect the user experience, and until we're satisfied that said DLL won't
> crash Firefox, a blocklisting is warranted.
> 
> From what I understand, we're seeing high levels of crashes in 53. 
> 
> r=me for DLL blocklisting in 53 until we're satisfied the problem has been
> addressed and doesn't impact Firefox and its users. 
> 
> 
> 
> (In reply to Charles Jones from comment #42)
> > Also because we did not test the new extension well for millions of users,
> > it would be better to have a backup SDK version for Firefox 53 for those
> > users who may face some rare compatibility problems with web extension.
> > Thus, you may disable our DLLs completely in Firefox 54. It should give us
> > enough time for smooth transition and for possible bug fixes.
Version 6.27.3 is the latest one on AMO, and it currently has strict compatibility set to 52. Older versions are hardblocked on 53 and above. Is version 6.27.3.1 of the add-on pending upload, or are you using a different ID?

Per comment #60 and the policy [1] we published months ago, I think blocking on 53 is the best way to go.

[1] https://blog.mozilla.org/addons/2017/01/24/preventing-add-ons-third-party-software-from-loading-dlls-into-firefox/
Comment on attachment 8855046 [details] [diff] [review]
bug1333486v3-beta.patch

Blocklisting for 53 sounds correct to me for this situation. Thanks. 
I'd like to get this into beta 10 build today if possible; if not it should land before the merge for the 53 RC build.
Attachment #8855046 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
We have 6.28 version of extension for Firefox >= 53 with another extension ID

https://addons.mozilla.org/en-US/developers/addon/idm-integration-module9/versions

This is webextension add-on. It does not load any DLLs. We are testing it with users of Firefox 53 beta, and with Firefox 54, 55

We may change its ID to old one in the future, and because we have updateUrl in install.rdf of old SDK add-on, it can be updated automatically

Also we may release updates 6.27.3.2 etc... for Firefox 52 ESR with the same old ID during ESR entire life cycle

(In reply to Jorge Villalobos [:jorgev] from comment #63)
> Version 6.27.3 is the latest one on AMO, and it currently has strict
> compatibility set to 52. Older versions are hardblocked on 53 and above. Is
> version 6.27.3.1 of the add-on pending upload, or are you using a different
> ID?
though the signatures have significantly reduced, there are still a number of crashes with involvement of the older idmcchandler5.dll - i'll add an update to the blocklist entry.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Attachment #8853094 - Attachment is obsolete: true
Attachment #8855046 - Attachment is obsolete: true
Attachment #8862898 - Flags: review?(mcastelluccio)
Comment on attachment 8862898 [details] [diff] [review]
bug1333486v4.patch

Review of attachment 8862898 [details] [diff] [review]:
-----------------------------------------------------------------

LGTM, but it needs to be reviewed from bsmedberg/aklotz too.
Attachment #8862898 - Flags: review?(mcastelluccio)
Attachment #8862898 - Flags: review?(benjamin)
Attachment #8862898 - Flags: review?(aklotz)
Attachment #8862898 - Flags: review+
Attachment #8862898 - Flags: review?(benjamin)
Attachment #8862898 - Flags: review?(aklotz)
Attachment #8862898 - Flags: review+
Keywords: checkin-needed
Comment on attachment 8862898 [details] [diff] [review]
bug1333486v4.patch

Approval Request Comment
[Feature/Bug causing the regression]: external extension hooking into firefox
[User impact if declined]: crashes
[Is this code covered by automated tests?]: n/a
[Has the fix been verified in Nightly?]: no, but the prior iteration of the patch  has gone into release now and fixed the crashes it was targetting. this is adding another (older & less common) dll for blocklisting in addition to that
[Needs manual test from QE? If yes, steps to reproduce]: n/a
[List of other uplifts needed for the feature/fix]: n/a
[Is the change risky?]: no
[Why is the change risky/not risky?]: the patch is making use of the
purpose-built dll blocklisting mechanism and just adding a new dll entry there
[String changes made/needed]: n/a
Attachment #8862898 - Flags: approval-mozilla-beta?
Pushed by ryanvm@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/9a72b71d8820
Add idmcchandler5.dll/idmcchandler5_64.dll to the blocklist. r=marco
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/9a72b71d8820
Status: REOPENED → RESOLVED
Closed: 3 years ago3 years ago
Resolution: --- → FIXED
Comment on attachment 8862898 [details] [diff] [review]
bug1333486v4.patch

Block dlls to avoid crash. Beta54+. Should be in 54 beta 5.
Attachment #8862898 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
(In reply to [:philipp] from comment #70)
> [Is this code covered by automated tests?]: n/a
> [Has the fix been verified in Nightly?]: no, but the prior iteration of the
> patch  has gone into release now and fixed the crashes it was targetting.
> this is adding another (older & less common) dll for blocklisting in
> addition to that
> [Needs manual test from QE? If yes, steps to reproduce]: n/a

Setting qe-verify- based on Philip's assessment on manual testing needs.
Flags: qe-verify-
Component: Blocklist Policy Requests → Blocklist Implementation
Component: Blocklist Implementation → Blocklist Policy Requests
You need to log in before you can comment on or make changes to this bug.