1384106 - Implement out-of-process accessibility tool block listing

Status: NEW

(Core :: Disability Access APIs, enhancement, P3)

Description

[Jim Mathies [:jimm]]

We have the ability now to detect the binaries of 3rd party applications that access our accessibility apis. Implement a method by which we can block these exe using a hard coded list, similar to our current dll blocklisting service. We should look at combining these two blocking mechanisms if we can.

Comment 1

[(No longer employed by Mozilla) Aaron Klotz]

http://searchfox.org/mozilla-central/rev/3a3af33f513071ea829debdfbc628caebcdf6996/accessible/windows/msaa/LazyInstantiator.cpp#254

Is where this should hook in.

Comment 2

[Jim Mathies [:jimm]]

nice!

Comment 3

[Carl Corcoran [:ccorcoran]]

Do we have any real-world use-cases for this kind of blocking?

Comment 4

[(No longer employed by Mozilla) Aaron Klotz]

We certainly do!

Comment 5

[David Bolter [:davidb] (NeedInfo me for attention)]

Yep. E.g. "hao123.1.0.0.1111.exe" and "Netfixs.exe"

I think one open question is whether we can block malicious software while allowing assistive software access.

A related effort happening in parallel here is to help users discover that software is using this API vector (for good or bad), and to allow the user to block access.

Comment 6

[(No longer employed by Mozilla) Aaron Klotz]

(In reply to David Bolter [:davidb] (NeedInfo me for attention) from comment #5)
> I think one open question is whether we can block malicious software while
> allowing assistive software access.

That should not be a problem. Legitimate ATs will be able to get past the check and instantiate a11y.

Comment 7

[Carl Corcoran [:ccorcoran]]

Small quirk on this before it can be implemented: LazyInstantiator::mWeakAccessible is effectively a singleton, which means if the first client is blocked (i.e. given a dummy object) then all future clients get the fake dummy object. Conversely if the first client is not blocked, then the block check is never called again, circumventing the blocking.

The natural solution is to check if a client is blocked upon every single instantiation. By itself that's going to kill performance, calling GetClientExecutableName every time a node in the tree is requested. We can keep a cache that maps threadID to isBlocked, though PID/TID get reused so it wouldn't be reliable. But maybe using GetThreadTimes(), mapping {threadID, startTime} would be reliable.

Thoughts anyone on this solution?

Comment 8

[Carl Corcoran [:ccorcoran]]

Attachment: 1384106.patch

Attached a working patch. This effectively blocks based on client EXE name, while still allowing other EXE clients. I'm working around the singleton issue with the caching mechanism explained above (caching the heavy logic keyed by threadID + thread start time).

I will also attach the source to a small EXE msaa client app I used for testing.

I am at the end of my contract with Mozilla so I will leave this here for the next person to pick up. It works but needs a couple things before finalizing:

1. The client cache is never cleaned. If for example an msaa client is creating IAccessible objects from many threads, that could pile up. Could easily be solved by doing a sort of garbage collection every so often, checking whether the cached thread IDs are still actively running threads.

2. Blocked client info should ideally be dynamically managed, not hard-coded.

3. Real-world testing should be done. I seeded the blocked client list with 2 EXEs, but only tested with my fake test app. The *actual* clients-to-be-blocked should be tested.

4. Regression tests should be created.

And last thing to note: Currently, there's a comment @ https://searchfox.org/mozilla-central/rev/f2a1911ad310bf8651f342d719e4f4ca0a7b9bfb/accessible/windows/msaa/LazyInstantiator.cpp#475

> // If we don't want a real root, let's resolve a fake one.

This makes sense but I found two issues where it relates to client blocking:

1. Because of the singleton nature of these objects, you either get a fake object or a real object forever. If a fake object is used, then all msaa clients will get the fake object. If we want to give blocked clients a fake object, then this behavior will need to be refined. For this reason, my patch returns E_FAIL for blocked clients, which from my local testing is stable.

2. On my local machine, fake objects were never actually succeeding. This call:

> ::DefWindowProc(mHwnd, WM_GETOBJECT, flags, static_cast<LPARAM>(OBJID_CLIENT));

Was returning NULL and causing the next line to always fail. It didn't directly affect this blocking feature but seems relevant.

Comment 9

[Carl Corcoran [:ccorcoran]]

Attachment: msaablockingtest.cpp

Here's the source to a little simple msaa client for testing purposes.

Comment 10

[(No longer employed by Mozilla) Aaron Klotz]

(In reply to Carl Corcoran [:ccorcoran] (unavailable for NI) from comment #8)
> 2. On my local machine, fake objects were never actually succeeding. This
> call:
> 
> > ::DefWindowProc(mHwnd, WM_GETOBJECT, flags, static_cast<LPARAM>(OBJID_CLIENT));
> 
> Was returning NULL and causing the next line to always fail. It didn't
> directly affect this blocking feature but seems relevant.

We're aware of that but don't really care. The intent there was to future-proof this feature by simulating a WM_GETOBJECT being handled by DefWindowProc, as if we didn't support MSAA.

But the current implementation in DefWindowProc doesn't actually return a placeholder object, it just fails out. So this issue is less "something is broken here" and more "we're trying to be a good citizen and do the right thing according to the docs in case the underlying implementation changes."

Comment 11

[David Bolter [:davidb] (NeedInfo me for attention)]

I'm going to assume we wouldn't take this for 57 this late in cycle. Marking status.

Comment 12

[(No longer employed by Mozilla) Aaron Klotz]

I'm marking this as P3 for InjectEject work.