Skip to content

Merge GHSA-rmq3-vvhq-gp32 to beta#14031

Merged
feerrenrut merged 3 commits into
betafrom
beta-GHSA-rmq3-vvhq-gp32
Aug 18, 2022
Merged

Merge GHSA-rmq3-vvhq-gp32 to beta#14031
feerrenrut merged 3 commits into
betafrom
beta-GHSA-rmq3-vvhq-gp32

Conversation

@seanbudd

@seanbudd seanbudd commented Aug 18, 2022

Copy link
Copy Markdown
Member

Merge GHSA-rmq3-vvhq-gp32 to beta

Must be merge commit, not squash merge

See security advisory:
GHSA-rmq3-vvhq-gp32

There are two security issues fixed by this change:
1. Access to NVDA python console and file explorer while on the lock screen.
   After opening and focusing the Windows Magnifier (and potentially other
   windows) from the `LockApp` (the lockscreen) NVDA no longer recognized that
   the lock screen was open.
  * To determine if NVDA is operating on the lock screen, it checked if the
   `LockApp` is the foreground window.
  * However, when the Magnifier has focus, the `LockApp` is not the foreground
    window.
  * The `LockApp` can still be open behind the magnifier, and should be
    preventing access to the logged in user's Windows profile.
  * If NVDA doesn't know that it is operating on the lock screen, it won't
    prevent access to tools that give access to the user's profile (E.G. NVDA
    python console)

2. Although not easy to reproduce, it was possible to report certain
   information about open applications from the lockscreen.
   * NVDA's `api` module is responsible for caching various system state used
     by NVDA, this is done through methods like `setForegroundObject`,
     `setFocusObject`, etc.
   * These methods rely on `_isSecureObjectWhileLockScreenActivated` to check
     if an object is permitted for use.
   * Secure objects are objects which are not intended to be available while
     Windows is locked.
   * These functions (`setForegroundObject`, `setFocusObject`, etc.) returned
     `True` if setting the object was a success and `False` otherwise.
   * However, consumers of these functions weren't observing the return value
     and the "secure objects" were used/reported even if the `setX` method in
     `api` failed.

* It is no longer possible to run a python console from the lockscreen.
* It is no longer possible to report information from below the lockscreen using
  object navigation.
  * `NVDAObjects` which fail to be set by the `api` module will not be read, and
  should be treated as if they do not exist by NVDA.
* Considering security precautions has been added to PR templates.

NVDA now determines if Windows is locked based on Windows Session notifications.
https://docs.microsoft.com/en-us/windows/win32/api/wtsapi32/nf-wtsapi32-wtsregistersessionnotification

When locked:
* A `LockApp` overlay class is applied to NVDA objects, to ensure they cannot
  read information on the desktop via object navigation.
* Objects cannot be navigated to outside the active foreground process
  (i.e. visible on the lockscreen).
* Only a whitelist of NVDA scripts/gestures are allowed.

The `api` module functions which use `_isSecureObjectWhileLockScreenActivated`
were identified:
* `setNavigatorObject`
* `setMouseObject`
* `setFocusObject`
* `setForegroundObject`

Usages of these `api` functions were found and inspected.
Ensured the return value is now observed; on failure to set the object, do not
proceed to use the object.

Additionally, as a precaution, other widely used functions that receive an
`NVDAObject` have had protections added:
* `getObjectPropertiesSpeech/getObjectSpeech` checks the object and now returns
an empty speech sequence if it is secure.
* Similar checks have been provided for eventHandler and braille objects.

Finally, the task list / switcher (`alt+tab`) window needs to be explicitly
added to an allow list for interaction while the lock screen is open because
it does not does not become the foreground process on the lock screen.
This makes it impossible to confirm that it is 'above' the lockscreen.
@seanbudd seanbudd requested a review from a team as a code owner August 18, 2022 03:43
@seanbudd seanbudd requested review from feerrenrut and removed request for a team August 18, 2022 03:43
feerrenrut
feerrenrut previously approved these changes Aug 18, 2022
@seanbudd seanbudd force-pushed the beta-GHSA-rmq3-vvhq-gp32 branch from d677a49 to ddea58c Compare August 18, 2022 05:05
@seanbudd seanbudd requested a review from feerrenrut August 18, 2022 05:15
@feerrenrut feerrenrut merged commit 54e95bf into beta Aug 18, 2022
@feerrenrut feerrenrut deleted the beta-GHSA-rmq3-vvhq-gp32 branch August 18, 2022 06:04
@nvaccessAuto nvaccessAuto added this to the 2022.4 milestone Aug 18, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants