Merge pull request from GHSA-585m-rpvv-93qg

Addresses GHSA-585m-rpvv-93qg

Summary of the issue:
NVDA introduced the report dev info script as a safe script for the lock screen in 2021.3.2 via #13328.
This was under the assumption that the log viewer never shows up on the lock screen.

However, using certain steps, the log viewer can be interacted with on the lock screen.
Further steps allow opening the NVDA python console, allowing arbitrary code execution.

Description of user facing changes
The devInfo script (open the log viewer and report navigator object information) is no longer available on the lock screen.

Description of development approach
Remove devInfo from safe scripts

Review the security of other scripts in safe scripts.

Added additional security protection to ScreenExplorer used by touch interaction, as well as setting the review position with api.setReviewPosition.

Testing strategy:
Test with a self-signed build the STR in GHSA-585m-rpvv-93qg

source/api.py

@@ -226,20 +226,19 @@ def getReviewPosition() -> textInfos.TextInfo:
 
 
 def setReviewPosition(
-		reviewPosition,
-		clearNavigatorObject=True,
-		isCaret=False,
-		isMouse=False
-):
+		reviewPosition: textInfos.TextInfo,
+		clearNavigatorObject: bool = True,
+		isCaret: bool = False,
+		isMouse: bool = False
+) -> bool:
 	"""Sets a TextInfo instance as the review position.
-	@param clearNavigatorObject: if  true, It sets the current navigator object to C{None}.
+	@param clearNavigatorObject: if True, It sets the current navigator object to C{None}.
 		In that case, the next time the navigator object is asked for it fetches it from the review position.
-	@type clearNavigatorObject: bool
 	@param isCaret: Whether the review position is changed due to caret following.
-	@type isCaret: bool
 	@param isMouse: Whether the review position is changed due to mouse following.
-	@type isMouse: bool
 	"""
+	if _isSecureObjectWhileLockScreenActivated(reviewPosition.obj):
+		return False
 	globalVars.reviewPosition=reviewPosition.copy()
 	globalVars.reviewPositionObj=reviewPosition.obj
 	if clearNavigatorObject: globalVars.navigatorObject=None
@@ -254,6 +253,7 @@ def setReviewPosition(
 	else:
 		visionContext = vision.constants.Context.REVIEW
 	vision.handler.handleReviewMove(context=visionContext)
+	return True
 
 
 def getNavigatorObject() -> NVDAObjects.NVDAObject:

source/screenExplorer.py

@@ -8,6 +8,7 @@
 import textInfos
 import locationHelper
 import speech
+from utils.security import _isSecureObjectWhileLockScreenActivated
 
 class ScreenExplorer(object):
 
@@ -17,7 +18,16 @@ def __init__(self):
 		self._obj=None
 		self._pos=None
 
-	def moveTo(self,x,y,new=False,unit=textInfos.UNIT_LINE):
+	# C901 'moveTo' is too complex
+	# Note: when working on moveTo, look for opportunities to simplify
+	# and move logic out into smaller helper functions.
+	def moveTo(  # noqa: C901
+			self,
+			x: int,
+			y: int,
+			new: bool = False,
+			unit: str = textInfos.UNIT_LINE,
+	) -> None:
 		obj=api.getDesktopObject().objectFromPoint(x,y)
 		prevObj=None
 		while obj  and obj.beTransparentToMouse:
@@ -55,11 +65,21 @@ def moveTo(self,x,y,new=False,unit=textInfos.UNIT_LINE):
 		if pos and self.updateReview:
 			api.setReviewPosition(pos)
 		speechCanceled=False
-		if hasNewObj:
+		if hasNewObj and not _isSecureObjectWhileLockScreenActivated(obj):
 			speech.cancelSpeech()
 			speechCanceled=True
 			speech.speakObject(obj)
-		if pos  and (new or not self._pos or pos.__class__!=self._pos.__class__ or pos.compareEndPoints(self._pos,"startToStart")!=0 or pos.compareEndPoints(self._pos,"endToEnd")!=0):
+		if (
+			pos
+			and (
+				new
+				or not self._pos
+				or pos.__class__ != self._pos.__class__
+				or pos.compareEndPoints(self._pos, "startToStart") != 0
+				or pos.compareEndPoints(self._pos, "endToEnd") != 0
+			)
+			and not _isSecureObjectWhileLockScreenActivated(pos.obj)
+		):
 				self._pos=pos
 				if not speechCanceled:
 					speech.cancelSpeech()

source/utils/security.py

@@ -28,10 +28,40 @@ def getSafeScripts() -> Set["scriptHandler._ScriptFunctionT"]:
 	# and it might be needed by global maps.
 	from globalCommands import commands
 	return {
+		# The focus object should not cache secure content
+		# due to handling in `api.setFocusObject`.
 		commands.script_reportCurrentFocus,
+
+		# Reports the foreground window.
+		# The foreground object should not cache secure content
+		# due to handling in `api.setForegroundObject`.
 		commands.script_title,
+
+		# Reports system information that should be accessible from the lock screen.
 		commands.script_dateTime,
 		commands.script_say_battery_status,
+
+		# Mouse navigation is required to ensure controls
+		# on the lock screen are accessible.
+		# Preventing mouse navigation outside the lock screen
+		# is handled using `api.setMouseObject` and `api.setNavigatorObject`.
+		commands.script_moveMouseToNavigatorObject,
+		commands.script_moveNavigatorObjectToMouse,
+		commands.script_leftMouseClick,
+		commands.script_rightMouseClick,
+
+		# Braille commands are safe, and required to interact
+		# on the lock screen using braille.
+		commands.script_braille_scrollBack,
+		commands.script_braille_scrollForward,
+		commands.script_braille_routeTo,
+		commands.script_braille_previousLine,
+		commands.script_braille_nextLine,
+
+		# Object navigation is required to ensure controls
+		# on the lock screen are accessible.
+		# Preventing object navigation outside the lock screen
+		# is handled in `api.setNavigatorObject` and by applying `LockScreenObject`.
 		commands.script_navigatorObject_current,
 		commands.script_navigatorObject_currentDimensions,
 		commands.script_navigatorObject_toFocus,
@@ -40,7 +70,13 @@ def getSafeScripts() -> Set["scriptHandler._ScriptFunctionT"]:
 		commands.script_navigatorObject_next,
 		commands.script_navigatorObject_previous,
 		commands.script_navigatorObject_firstChild,
-		commands.script_navigatorObject_devInfo,
+		commands.script_navigatorObject_nextInFlow,
+		commands.script_navigatorObject_previousInFlow,
+
+		# Moving the review cursor is required to ensure controls
+		# on the lock screen are accessible.
+		# Preventing review cursor navigation outside the lock screen
+		# is handled in `api.setReviewPosition`.
 		commands.script_review_activate,
 		commands.script_review_top,
 		commands.script_review_previousLine,
@@ -56,21 +92,16 @@ def getSafeScripts() -> Set["scriptHandler._ScriptFunctionT"]:
 		commands.script_review_nextCharacter,
 		commands.script_review_endOfLine,
 		commands.script_review_sayAll,
-		commands.script_braille_scrollBack,
-		commands.script_braille_scrollForward,
-		commands.script_braille_routeTo,
-		commands.script_braille_previousLine,
-		commands.script_braille_nextLine,
-		commands.script_navigatorObject_nextInFlow,
-		commands.script_navigatorObject_previousInFlow,
-		commands.script_touch_changeMode,
-		commands.script_touch_newExplore,
-		commands.script_touch_explore,
-		commands.script_touch_hoverUp,
-		commands.script_moveMouseToNavigatorObject,
-		commands.script_moveNavigatorObjectToMouse,
-		commands.script_leftMouseClick,
-		commands.script_rightMouseClick,
+
+		# Using the touch screen is required to ensure controls
+		# on the lock screen are accessible.
+		# Preventing touch navigation outside the lock screen
+		# is handled in `screenExplorer.ScreenExplorer.moveTo`.
+		commands.script_touch_changeMode,  # cycles through available touch screen modes
+		commands.script_touch_newExplore,  # tap gesture, reports content under the finger
+		commands.script_touch_explore,  # hover gesture, reports content changes under the finger
+		commands.script_touch_hoverUp,  # hover up gesture, fixes a situation with touch typing
+		# commands.script_touch_rightClick, TODO: consider adding, was this missed previously?
 	}
 
 

user_docs/en/changes.t2t

@@ -4,11 +4,14 @@ What's New in NVDA
 %!includeconf: ../changes.t2tconf
 
 = 2022.2.3 =
-This is a patch release to fix an accidental API breakage introduced in 2022.2.1.
+This is a patch release to fix a security issue.
+This release also fixes an accidental API breakage introduced in 2022.2.1.
 
 == Bug Fixes ==
 - Fixed a bug where NVDA did not announce "Secure Desktop" when entering a secure desktop.
 This caused NVDA remote to not recognize secure desktops. (#14094)
+- Fixed an exploit where it was possible to open the NVDA python console via the log viewer on the lock screen.
+([GHSA-585m-rpvv-93qg https://github.com/nvaccess/nvda/security/advisories/GHSA-585m-rpvv-93qg])
 -
 
 = 2022.2.2 =