Closed
Bug 1348069
Opened 8 years ago
Closed 8 years ago
Robustness against third-party tampering with MSAA registry settings
Categories
(Core :: Disability Access APIs, enhancement)
Tracking
()
RESOLVED
FIXED
mozilla55
Tracking | Status | |
---|---|---|
firefox55 | --- | fixed |
People
(Reporter: bugzilla, Assigned: bugzilla)
References
Details
(Whiteboard: aes+)
Attachments
(1 file, 1 obsolete file)
(deleted),
patch
|
jimm
:
review+
gps
:
review+
|
Details | Diff | Splinter Review |
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.
Assignee | ||
Comment 1•8 years ago
|
||
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.
Attachment #8848245 -
Flags: review?(jmathies)
Assignee | ||
Comment 2•8 years ago
|
||
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•8 years ago
|
||
Are we good about IAccessible2, ISimpleDOMNode, IAccessibleEx stuff?
Does the approach interfere with Adobe's plugin accessibility, i.e is it still accessible?
Assignee | ||
Comment 4•8 years ago
|
||
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•8 years ago
|
||
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
Attachment #8848245 -
Flags: review?(jmathies) → review+
Assignee | ||
Comment 6•8 years ago
|
||
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•8 years ago
|
||
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.
Assignee | ||
Comment 8•8 years ago
|
||
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.
Assignee | ||
Comment 9•8 years ago
|
||
Attachment #8848245 -
Attachment is obsolete: true
Attachment #8852609 -
Flags: review?(jmathies)
Attachment #8852609 -
Flags: review?(gps)
Comment 10•8 years ago
|
||
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.
Attachment #8852609 -
Flags: review?(gps) → review+
Updated•8 years ago
|
Attachment #8852609 -
Flags: review?(jmathies) → review+
Assignee | ||
Comment 11•8 years ago
|
||
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•8 years ago
|
||
bugherder |
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
You need to log in
before you can comment on or make changes to this bug.
Description
•