Skip to content

Commit

Permalink
Merge pull request from GHSA-rmq3-vvhq-gp32
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
seanbudd authored and feerrenrut committed Aug 18, 2022
1 parent 6794032 commit d4de238
Show file tree
Hide file tree
Showing 31 changed files with 844 additions and 261 deletions.
1 change: 1 addition & 0 deletions .github/PULL_REQUEST_TEMPLATE.md
Original file line number Diff line number Diff line change
Expand Up @@ -52,3 +52,4 @@ https://github.com/nvaccess/nvda/blob/master/devDocs/githubPullRequestTemplateEx
- Low Vision
- Different web browsers
- Localization in other languages / culture than English
- [ ] Security precautions taken.
11 changes: 11 additions & 0 deletions devDocs/githubPullRequestTemplateExplanationAndExamples.md
Original file line number Diff line number Diff line change
Expand Up @@ -172,3 +172,14 @@ Discuss under "testing strategy" heading:
- Localization in other languages / culture than English
- When one of these can not be supported with this change,
highlight it under the "Known issues" heading

### Security precautions taken
Windows allows NVDA to access secure information from a user's desktop while the lock screen is activated.
When handling NVDAObjects, code should check if an object should be available while the lock screen is activated.
This can be done checking `utils.security._isSecureObjectWhileLockScreenActivated`.
If this function returns `True`, code should not process the NVDAObject any further, and return gracefully.
It is important that information from the object does not reach the user.

Any code which should not performed on [secure screens](https://www.nvaccess.org/files/nvda/documentation/userGuide.html#SecureScreens), should check `globalVars.appArgs.secure` and return gracefully.
If this is a user initiated action `gui.blockAction` should be used to notify the user that the action cannot be performed.
This can be done by decorating the function with `@blockAction.when(blockAction.Context.SECURE_MODE)`.
12 changes: 6 additions & 6 deletions source/NVDAObjects/UIA/VisualStudio.py
Original file line number Diff line number Diff line change
Expand Up @@ -25,12 +25,12 @@ def event_UIA_elementSelected(self):
# Therefore, if speech wouldn't be cancelled,
# selection announcements would queue up when changing selection rapidly.
speech.cancelSpeech()
api.setNavigatorObject(self, isFocus=True)
self.reportFocus()
# Display results as flash messages.
braille.handler.message(braille.getPropertiesBraille(
name=self.name, role=self.role, positionInfo=self.positionInfo, description=self.description
))
if api.setNavigatorObject(self, isFocus=True):
self.reportFocus()
# Display results as flash messages.
braille.handler.message(braille.getPropertiesBraille(
name=self.name, role=self.role, positionInfo=self.positionInfo, description=self.description
))


class IntelliSenseList(UIA):
Expand Down
19 changes: 7 additions & 12 deletions source/NVDAObjects/UIA/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,20 +10,15 @@
Optional,
Dict,
)
from ctypes import byref
from ctypes.wintypes import POINT, RECT
from ctypes.wintypes import POINT
from comtypes import COMError
from comtypes.automation import VARIANT
import time
import weakref
import numbers
import colors
import languageHandler
import UIAHandler
import UIAHandler.customProps
import UIAHandler.customAnnotations
import globalVars
import eventHandler
import controlTypes
from controlTypes import TextPosition
import config
Expand Down Expand Up @@ -2218,12 +2213,12 @@ def event_UIA_elementSelected(self):
focusControllerFor = api.getFocusObject().controllerFor
if len(focusControllerFor) > 0 and focusControllerFor[0].appModule is self.appModule and self.name:
speech.cancelSpeech()
api.setNavigatorObject(self, isFocus=True)
self.reportFocus()
# Display results as flash messages.
braille.handler.message(braille.getPropertiesBraille(
name=self.name, role=self.role, positionInfo=self.positionInfo
))
if api.setNavigatorObject(self, isFocus=True):
self.reportFocus()
# Display results as flash messages.
braille.handler.message(braille.getPropertiesBraille(
name=self.name, role=self.role, positionInfo=self.positionInfo
))

# NetUIDropdownAnchor comboBoxes (such as in the MS Office Options dialog)
class NetUIDropdownAnchor(UIA):
Expand Down
47 changes: 37 additions & 10 deletions source/NVDAObjects/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@
import brailleInput
import locationHelper
import aria
from winAPI.sessionTracking import isWindowsLocked


class NVDAObjectTextInfo(textInfos.offsets.OffsetsTextInfo):
Expand Down Expand Up @@ -108,6 +109,12 @@ def __call__(self,chooseBestAPI=True,**kwargs):
log.exception(f"Exception in chooseNVDAObjectOverlayClasses for {plugin}")
pass

# After all other mutation has finished,
# add LockScreenObject if Windows is locked.
# LockScreenObject must become the first class to be resolved,
# i.e. insertion order of 0.
self._insertLockScreenObject(clsList)

# Determine the bases for the new class.
bases=[]
for index in range(len(clsList)):
Expand Down Expand Up @@ -166,6 +173,16 @@ def clearDynamicClassCache(cls):
"""
cls._dynamicClassCache.clear()

def _insertLockScreenObject(self, clsList: typing.List["NVDAObject"]) -> None:
"""
Inserts LockScreenObject to the start of the clsList if Windows is locked.
"""
from .lockscreen import LockScreenObject
if isWindowsLocked():
# This must be resolved first to prevent object navigation outside of the lockscreen.
clsList.insert(0, LockScreenObject)


class NVDAObject(documentBase.TextContainerObject, baseObject.ScriptableObject, metaclass=DynamicNVDAObjectType):
"""NVDA's representation of a single control/widget.
Every widget, regardless of how it is exposed by an application or the operating system, is represented by a single NVDAObject instance.
Expand Down Expand Up @@ -255,19 +272,26 @@ def kwargsFromSuper(cls,kwargs,relation=None):
@rtype: boolean
"""
raise NotImplementedError


def findOverlayClasses(self, clsList):
"""Chooses overlay classes which should be added to this object's class structure after the object has been initially instantiated.
After an NVDAObject class (normally an API-level class) is instantiated, this method is called on the instance to choose appropriate overlay classes.
def findOverlayClasses(self, clsList: typing.List["NVDAObject"]) -> None:
"""
Chooses overlay classes which should be added to this object's class structure,
after the object has been initially instantiated.
After an NVDAObject class (normally an API-level class) is instantiated,
this method is called on the instance to choose appropriate overlay classes.
This method may use properties, etc. on the instance to make this choice.
The object's class structure is then mutated to contain these classes.
L{initOverlayClass} is then called for each class which was not part of the initially instantiated object.
This process allows an NVDAObject to be dynamically created using the most appropriate NVDAObject subclass at each API level.
Classes should be listed with subclasses first. That is, subclasses should generally call super and then append their own classes to the list.
For example: Called on an IAccessible NVDAObjectThe list might contain DialogIaccessible (a subclass of IAccessible), Edit (a subclass of Window).
Classes should be listed with subclasses first.
That is, subclasses should generally call super and then append their own classes to the list.
For example: Called on an IAccessible NVDAObject, the list might contain:
"DialogIAccessible (a subclass of IAccessible), Edit (a subclass of Window)".
@param clsList: The list of classes, which will be modified by this method if appropriate.
@type clsList: list of L{NVDAObject}
"""
clsList.append(NVDAObject)

Expand Down Expand Up @@ -969,9 +993,11 @@ def _get_positionInfo(self):
"""
return {}

def _get_processID(self):
"""Retrieves an identifyer of the process this object is a part of.
@rtype: int
#: Type definition for auto prop '_get_processID'
processID: int

def _get_processID(self) -> int:
"""Retrieves an identifier of the process this object is a part of.
"""
raise NotImplementedError

Expand Down Expand Up @@ -1281,6 +1307,7 @@ def _get_devInfo(self) -> typing.List[str]: # noqa: C901
info.append("name: %s" % ret)
ret = self.role
info.append("role: %s" % ret)
info.append(f"processID: {self.processID}")
try:
ret = repr(self.roleText)
except Exception as e:
Expand Down
6 changes: 3 additions & 3 deletions source/NVDAObjects/behaviors.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
# A part of NonVisual Desktop Access (NVDA)
# This file is covered by the GNU General Public License.
# See the file COPYING for more details.
# Copyright (C) 2006-2020 NV Access Limited, Peter Vágner, Joseph Lee, Bill Dengler
# Copyright (C) 2006-2022 NV Access Limited, Peter Vágner, Joseph Lee, Bill Dengler

"""Mix-in classes which provide common behaviour for particular types of controls across different APIs.
Behaviors described in this mix-in include providing table navigation commands for certain table rows, terminal input and output support, announcing notifications and suggestion items and so on.
Expand Down Expand Up @@ -591,8 +591,8 @@ def _moveToColumn(self, obj):
if obj is not self:
# Use the focused copy of the row as the parent for all cells to make comparison faster.
obj.parent = self
api.setNavigatorObject(obj)
speech.speakObject(obj, reason=controlTypes.OutputReason.FOCUS)
if api.setNavigatorObject(obj):
speech.speakObject(obj, reason=controlTypes.OutputReason.FOCUS)

def _moveToColumnNumber(self, column):
child = column - 1
Expand Down
36 changes: 36 additions & 0 deletions source/NVDAObjects/lockscreen.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
# A part of NonVisual Desktop Access (NVDA)
# This file is covered by the GNU General Public License.
# See the file COPYING for more details.
# Copyright (C) 2022 NV Access Limited

from typing import (
Optional
)

from NVDAObjects import NVDAObject


class LockScreenObject(NVDAObject):
"""
Prevent users from object navigating outside of the lock screen.
While usages of `_isSecureObjectWhileLockScreenActivated` in the api module prevent
the user from moving to the object, this overlay class prevents reading neighbouring objects.
"""

def _get_next(self) -> Optional[NVDAObject]:
nextObject = super()._get_next()
if nextObject and nextObject.appModule.appName == self.appModule.appName:
return nextObject
return None

def _get_previous(self) -> Optional[NVDAObject]:
previousObject = super()._get_previous()
if previousObject and previousObject.appModule.appName == self.appModule.appName:
return previousObject
return None

def _get_parent(self) -> Optional[NVDAObject]:
parentObject = super()._get_parent()
if parentObject and parentObject.appModule.appName == self.appModule.appName:
return parentObject
return None
3 changes: 3 additions & 0 deletions source/NVDAObjects/window/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -178,6 +178,9 @@ def _get_name(self):
def _get_role(self):
return controlTypes.Role.WINDOW

# type information for auto property _get_windowClassName
windowClassName: str

def _get_windowClassName(self):
if hasattr(self,"_windowClassName"):
return self._windowClassName
Expand Down
55 changes: 16 additions & 39 deletions source/api.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@
"""General functions for NVDA
Functions should mostly refer to getting an object (NVDAObject) or a position (TextInfo).
"""

import enum
import typing

import config
Expand All @@ -27,45 +29,12 @@
import appModuleHandler
import cursorManager
from typing import Any, Optional
from utils.security import _isSecureObjectWhileLockScreenActivated

if typing.TYPE_CHECKING:
import documentBase


def _isLockAppAndAlive(appModule: "appModuleHandler.AppModule"):
return appModule.appName == "lockapp" and appModule.isAlive


def _isSecureObjectWhileLockScreenActivated(obj: NVDAObjects.NVDAObject) -> bool:
"""
While Windows is locked, Windows 10 and 11 allow for object navigation outside of the lockscreen.
@return: C{True} if the Windows 10/11 lockscreen is active and C{obj} is outside of the lockscreen.
According to MS docs, "There is no function you can call to determine whether the workstation is locked."
https://docs.microsoft.com/en-gb/windows/win32/api/winuser/nf-winuser-lockworkstation
"""
runningAppModules = appModuleHandler.runningTable.values()
lockAppModule = next(filter(_isLockAppAndAlive, runningAppModules), None)
if lockAppModule is None:
return False

# The LockApp process might be kept alive
# So determine if it is active, check the foreground window
foregroundHWND = winUser.getForegroundWindow()
foregroundProcessId, _threadId = winUser.getWindowThreadProcessID(foregroundHWND)

isLockAppForeground = foregroundProcessId == lockAppModule.processID
isObjectOutsideLockApp = obj.appModule.processID != foregroundProcessId

if isLockAppForeground and isObjectOutsideLockApp:
if log.isEnabledFor(log.DEBUG):
devInfo = '\n'.join(obj.devInfo)
log.debug(f"Attempt at navigating to a secure object: {devInfo}")
return True
return False

#User functions

def getFocusObject() -> NVDAObjects.NVDAObject:
"""
Gets the current object with focus.
Expand Down Expand Up @@ -95,7 +64,8 @@ def setForegroundObject(obj: NVDAObjects.NVDAObject) -> bool:
- setLastForegroundEventObject
@param obj: the object that will be stored as the current foreground object
"""
if not isinstance(obj,NVDAObjects.NVDAObject):
if not isinstance(obj, NVDAObjects.NVDAObject):
log.error("Object is not a valid NVDAObject")
return False
if _isSecureObjectWhileLockScreenActivated(obj):
return False
Expand All @@ -114,7 +84,8 @@ def setFocusObject(obj: NVDAObjects.NVDAObject) -> bool: # noqa: C901
this function calls event_loseFocus on the object to notify it that it is losing focus.
@param obj: the object that will be stored as the focus object
"""
if not isinstance(obj,NVDAObjects.NVDAObject):
if not isinstance(obj, NVDAObjects.NVDAObject):
log.error("Object is not a valid NVDAObject")
return False
if _isSecureObjectWhileLockScreenActivated(obj):
return False
Expand Down Expand Up @@ -219,11 +190,15 @@ def getMouseObject():
return globalVars.mouseObject


def setMouseObject(obj: NVDAObjects.NVDAObject) -> None:
def setMouseObject(obj: NVDAObjects.NVDAObject) -> bool:
"""Tells NVDA to remember the given object as the object that is directly under the mouse"""
if not isinstance(obj, NVDAObjects.NVDAObject):
log.error("Object is not a valid NVDAObject")
return False
if _isSecureObjectWhileLockScreenActivated(obj):
return
return False
globalVars.mouseObject=obj
return True


def getDesktopObject() -> NVDAObjects.NVDAObject:
Expand Down Expand Up @@ -305,7 +280,7 @@ def getNavigatorObject() -> NVDAObjects.NVDAObject:
return globalVars.navigatorObject


def setNavigatorObject(obj: NVDAObjects.NVDAObject, isFocus: bool = False) -> Optional[bool]:
def setNavigatorObject(obj: NVDAObjects.NVDAObject, isFocus: bool = False) -> bool:
"""Sets an object to be the current navigator object.
Navigator objects can be used to navigate around the operating system (with the numpad),
without moving the focus.
Expand All @@ -316,6 +291,7 @@ def setNavigatorObject(obj: NVDAObjects.NVDAObject, isFocus: bool = False) -> Op
"""

if not isinstance(obj, NVDAObjects.NVDAObject):
log.error("Object is not a valid NVDAObject")
return False
if _isSecureObjectWhileLockScreenActivated(obj):
return False
Expand All @@ -333,6 +309,7 @@ def setNavigatorObject(obj: NVDAObjects.NVDAObject, isFocus: bool = False) -> Op
globalVars.reviewPosition=obj.treeInterceptor.makeTextInfo(textInfos.POSITION_CARET)
globalVars.reviewPositionObj=globalVars.reviewPosition
eventHandler.executeEvent("becomeNavigatorObject",obj,isFocus=isFocus)
return True

def isTypingProtected():
"""Checks to see if key echo should be suppressed because the focus is currently on an object that has its protected state set.
Expand Down
Loading

2 comments on commit d4de238

@seanbudd
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

NVDA alpha-26228,33ee524f and later includes an important security fix.
The security problem can be prevented by disabling the Windows lock screen.
NV Access strongly recommends disabling the lock screen.
Instructions to disable the lock screen can be found in the workaround section in GHSA-rmq3-vvhq-gp32.

This security fix is planned to be released via 2022.2.1 and 2022.3beta3 on Wednesday 24th August around 0:01am UTC.
Ideally, security fixes are released straight to production.
Unfortunately, the patch for this security fix is quite complex.
Additionally, there are many Windows OS versions and many sign-in configurations for Windows, especially in corporate environments.
As such, we are aiming for a short period of alpha testing before releasing this patch.

Please start by "smoke testing" the build and report results here:

  • Install the build
  • Lock your computer.
  • Test the lock screen to ensure it is still accessible for your needs.
  • Unlock your computer.
  • Restart your computer and log in.
  • Report either success/failure here.

We want to know even if no problems are found so we can gauge the level of testing that this change has received.
Please report any security issues found via [email protected].

The diff for this work can be reviewed in this commit.
More details can be found in the GitHub advisory GHSA-rmq3-vvhq-gp32.

Thanks in advance,
NV Access Software Developers

@Mohamed00
Copy link

@Mohamed00 Mohamed00 commented on d4de238 Aug 18, 2022 via email

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please sign in to comment.