1348069 - Robustness against third-party tampering with MSAA registry settings

Status: RESOLVED FIXED

(Core :: Disability Access APIs, enhancement)

Description

[(No longer employed by Mozilla) Aaron Klotz]

I just discovered that Adobe Acrobat overwrites IAccessible's typelib GUID with its own, thus pointing all MSAA clients to some Acrobat stuff.

Who knows whether it is uninstalled correctly... but some of our marshaling errors that we've been seeing in COM are error codes such as TYPE_E_LIBNOTREGISTERED and TYPE_E_CANTLOADLIBRARY, which is suggesting to me that the standard MSAA components are no longer accessible due to registry corruption either from Acrobat or some other application.

I think we should see if we can override these settings by using registration-free COM tags to point to the correct system GUIDs in xul.dll's manifest. That would force anything running inside xul.dll to ignore the registry and always use the right things.

Comment 1

[(No longer employed by Mozilla) Aaron Klotz]

Attachment: Add comInterfaceExternalProxyStub entries for IAccessible to firefox and plugin-container manifests

These entries override the registry, thus forcing COM to use the canonical GUIDs for resolving IAccessible.

Here's what the attributes in the comInterfaceExternalProxyStub tag represent:

iid is set to IID_IAccessible

proxyStubClsid32 is set to the GUID of the universal marshaler (i.e. the CLSID that consumes typelibs to generate proxies)

tlbid is the GUID of the IAccessible typelib that is embedded in oleacc.dll. This GUID is identical across all supported Windows versions.


Note that these changes are perfectly benign in non-a11y builds, so I didn't consider it necessary to worry about whether or not ACCESSIBILITY is defined (plus merging into these xml files would be an unnecessary pain in the butt).

I am updating both firefox and plugin-container manifests to ensure that a11y queries to plugins are also unaffected by registry tampering.


I was able to confirm that this patch eliminates the Adobe crap from being loaded in our process when a11y instantiates. Hopefully it helps with other corruption as well.

Comment 2

[(No longer employed by Mozilla) Aaron Klotz]

As a further note, I changed the exe manifests as opposed to adding it to a manifest for xul.dll because the contents of the exe's manifest is implicitly activated for the lifetime of the process, whereas dll manifests would require explicit (de)activation would would add significant complexity.

Comment 3

[alexander :surkov (:asurkov)]

Are we good about IAccessible2, ISimpleDOMNode, IAccessibleEx stuff?

Does the approach interfere with Adobe's plugin accessibility, i.e is it still accessible?

Comment 4

[(No longer employed by Mozilla) Aaron Klotz]

This really only affects interfaces that come with Windows and are registered in the system registry, so IAccessible and IAccessibleEx are the only ones that could potentially be affected.

Looking at my own system, it doesn't look to me like IAccessibleEx was touched by Acrobat.

This should not interfere with Flash accessibility.

Comment 5

[Jim Mathies [:jimm]]

Comment on attachment 8848245 [details] [diff] [review]
Add comInterfaceExternalProxyStub entries for IAccessible to firefox and plugin-container manifests

Interesting feature - some background for people interested. This looks good to me.
https://msdn.microsoft.com/en-us/library/ms973913.aspx

Comment 6

[(No longer employed by Mozilla) Aaron Klotz]

https://hg.mozilla.org/integration/mozilla-inbound/rev/f9ca413eb20998d25a649c66aea14d7e8f094531
Bug 1348069: Add comInterfaceExternalProxyStub entries for IAccessible to firefox and plugin-container manifests; r=jimm

Comment 7

[Phil Ringnalda (:philor)]

Backed out in https://hg.mozilla.org/integration/mozilla-inbound/rev/992ea4d9d583975beae869b0e68d34667bf4efe8 for https://treeherder.mozilla.org/logviewer.html#?job_id=86065328&repo=mozilla-inbound and https://treeherder.mozilla.org/logviewer.html#?job_id=86074902&repo=mozilla-inbound and https://treeherder.mozilla.org/logviewer.html#?job_id=86074923&repo=mozilla-inbound on Win8, and I presume https://treeherder.mozilla.org/logviewer.html#?job_id=86077409&repo=mozilla-inbound on Win7.

Comment 8

[(No longer employed by Mozilla) Aaron Klotz]

My bad, 64-bit doesn't use that typelib. I'll have to split this up so that these manifest changes only apply to 32-bit builds.

Comment 9

[(No longer employed by Mozilla) Aaron Klotz]

Attachment: Add comInterfaceExternalProxyStub entries for IAccessible to 32-bit firefox and plugin-container manifests

Comment 10

[Gregory Szorc [:gps]]

Comment on attachment 8852609 [details] [diff] [review]
Add comInterfaceExternalProxyStub entries for IAccessible to 32-bit firefox and plugin-container manifests

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

This looks good from a build system perspective.

Comment 11

[(No longer employed by Mozilla) Aaron Klotz]

https://hg.mozilla.org/integration/mozilla-inbound/rev/d4e6fb6fb40afe99415863dcdc771dcd6f6e9e02
Bug 1348069: Add comInterfaceExternalProxyStub entries for IAccessible to 32-bit firefox and plugin-container manifests; r=jimm, gps

Comment 12

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/d4e6fb6fb40a