Disable logging in secure mode

GitHub Advisory GHSA-354r-wr4v-cx28:

Summary:
With the --debug-logging NVDA command line option, it was possible to
enable debug logging in secure mode.
From a secure screen, it was possible to activate debug logging by
restarting NVDA and selecting "Restart with debug logging" in the Exit
Dialog.
This created an instance of NVDA performing debug logging from the
system profile, from a secure context.

Description of change
Prevent debug logging in secure mode.
Remove "Restart with debug logging" from the exit dialog options.
Remove "Install pending update" from the exit dialog options.

devDocs/technicalDesignOverview.md

@@ -67,6 +67,14 @@ Aside from accessibility and native APIs, Windows provides many functions which
 Information that can be obtained includes the class name of a window, the current foreground window and system battery status.
 Tasks that can be performed include moving/clicking the mouse and sending key presses.
 
+### Logging
+
+#### Logging in secure mode
+`logHandler.initialize` prevents logging in secure mode.
+This is because it is a security concern to log during secure mode (e.g. passwords are logged).
+To change this for testing, patch the source build.
+`nvda.log` files are then generated in the System profile's `%TEMP%` directory.
+
 ## NVDA Components
 NVDA is built with an extensible, modular, object oriented, abstract design.
 It is divided into several distinct components.

source/gui/__init__.py

@@ -1,7 +1,7 @@
 # -*- coding: UTF-8 -*-
 # A part of NonVisual Desktop Access (NVDA)
-# Copyright (C) 2006-2021 NV Access Limited, Peter Vágner, Aleksey Sadovoy, Mesar Hameed, Joseph Lee,
-# Thomas Stivers, Babbage B.V., Accessolutions, Julien Cochuyt
+# Copyright (C) 2006-2022 NV Access Limited, Peter Vágner, Aleksey Sadovoy, Mesar Hameed, Joseph Lee,
+# Thomas Stivers, Babbage B.V., Accessolutions, Julien Cochuyt, Cyrille Bougot
 # This file is covered by the GNU General Public License.
 # See the file COPYING for more details.
 
@@ -44,6 +44,8 @@
 import speechViewer
 import winUser
 import api
+from utils.displayString import DisplayStringEnum
+from enum import auto, unique
 
 try:
 	import updateCheck
@@ -608,6 +610,31 @@ def run():
 	wx.CallAfter(run)
 
 
+@unique
+class _ExitAction(DisplayStringEnum):
+
+	EXIT = auto()
+	RESTART = auto()
+	RESTART_WITH_ADDONS_DISABLED = auto()
+	RESTART_WITH_DEBUG_LOGGING_ENABLED = auto()
+	INSTALL_PENDING_UPDATE = auto()
+
+	@property
+	def _displayStringLabels(self):
+		return {
+			# Translators: An option in the combo box to choose exit action.
+			self.EXIT: _("Exit"),
+			# Translators: An option in the combo box to choose exit action.
+			self.RESTART: _("Restart"),
+			# Translators: An option in the combo box to choose exit action.
+			self.RESTART_WITH_ADDONS_DISABLED: _("Restart with add-ons disabled"),
+			# Translators: An option in the combo box to choose exit action.
+			self.RESTART_WITH_DEBUG_LOGGING_ENABLED: _("Restart with debug logging enabled"),
+			# Translators: An option in the combo box to choose exit action.
+			self.INSTALL_PENDING_UPDATE: _("Install pending update"),
+		}
+
+
 class ExitDialog(wx.Dialog):
 	_instance = None
 
@@ -638,21 +665,18 @@ def __init__(self, parent):
 
 		# Translators: The label for actions list in the Exit dialog.
 		labelText=_("What would you like to &do?")
-		self.actions = [
-			# Translators: An option in the combo box to choose exit action.
-			_("Exit"),
-			# Translators: An option in the combo box to choose exit action.
-			_("Restart")
-		]
+		allowedActions = list(_ExitAction)
 		# Windows Store version of NVDA does not support add-ons yet.
-		if not config.isAppX:
-			# Translators: An option in the combo box to choose exit action.
-			self.actions.append(_("Restart with add-ons disabled"))
-		# Translators: An option in the combo box to choose exit action.
-		self.actions.append(_("Restart with debug logging enabled"))
-		if updateCheck and updateCheck.isPendingUpdate():
-			# Translators: An option in the combo box to choose exit action.
-			self.actions.append(_("Install pending update"))
+		if config.isAppX:
+			allowedActions.remove(_ExitAction.RESTART_WITH_ADDONS_DISABLED)
+		# Changing debug level on secure screen is not allowed.
+		# Logging on secure screens could allow keylogging of passwords and retrieval from the SYSTEM user.
+		if globalVars.appArgs.secure:
+			allowedActions.remove(_ExitAction.RESTART_WITH_DEBUG_LOGGING_ENABLED)
+		# Installing updates should not happen in secure mode.
+		if globalVars.appArgs.secure or not (updateCheck and updateCheck.isPendingUpdate()):
+			allowedActions.remove(_ExitAction.INSTALL_PENDING_UPDATE)
+		self.actions = [i.displayString for i in allowedActions]
 		self.actionsList = contentSizerHelper.addLabeledControl(labelText, wx.Choice, choices=self.actions)
 		self.actionsList.SetSelection(0)
 
@@ -668,25 +692,22 @@ def __init__(self, parent):
 		self.CentreOnScreen()
 
 	def onOk(self, evt):
-		action=self.actionsList.GetSelection()
-		# Because Windows Store version of NVDA does not support add-ons yet, add 1 if action is 2 or above if this is such a case.
-		if action >= 2 and config.isAppX:
-			action += 1
-		if action == 0:
+		action = [a for a in _ExitAction if a.displayString == self.actionsList.GetStringSelection()][0]
+		if action == _ExitAction.EXIT:
 			WelcomeDialog.closeInstances()
 			if core.triggerNVDAExit():
 				# there's no need to destroy ExitDialog in this instance as triggerNVDAExit will do this
 				return
 			else:
 				log.error("NVDA already in process of exiting, this indicates a logic error.")
 				return
-		elif action == 1:
+		elif action == _ExitAction.RESTART:
 			queueHandler.queueFunction(queueHandler.eventQueue,core.restart)
-		elif action == 2:
+		elif action == _ExitAction.RESTART_WITH_ADDONS_DISABLED:
 			queueHandler.queueFunction(queueHandler.eventQueue,core.restart,disableAddons=True)
-		elif action == 3:
+		elif action == _ExitAction.RESTART_WITH_DEBUG_LOGGING_ENABLED:
 			queueHandler.queueFunction(queueHandler.eventQueue,core.restart,debugLogging=True)
-		elif action == 4:
+		elif action == _ExitAction.INSTALL_PENDING_UPDATE:
 			if updateCheck:
 				destPath, version, apiVersion, backCompatTo = updateCheck.getPendingUpdate()
 				from addonHandler import getIncompatibleAddons

source/logHandler.py

@@ -372,6 +372,23 @@ def _excepthook(*exc_info):
 def _showwarning(message, category, filename, lineno, file=None, line=None):
 	log.debugWarning(warnings.formatwarning(message, category, filename, lineno, line).rstrip(), codepath="Python warning")
 
+
+def _shouldDisableLogging() -> bool:
+	"""Disables logging based on command line options and if secure mode is active.
+	See NoConsoleOptionParser in nvda.pyw, #TODO and #8516.
+
+	Secure mode disables logging.
+	Logging on secure screens could allow keylogging of passwords and retrieval from the SYSTEM user.
+
+	* `--secure` overrides any logging preferences by disabling logging.
+	* `--debug-logging` or `--log-level=X` overrides the user config log level setting.
+	* `--debug-logging` and `--log-level=X` override `--no-logging`.
+	"""
+	logLevelOverridden = globalVars.appArgs.debugLogging or not globalVars.appArgs.logLevel == 0
+	noLoggingRequested = globalVars.appArgs.noLogging and not logLevelOverridden
+	return globalVars.appArgs.secure or noLoggingRequested
+
+
 def initialize(shouldDoRemoteLogging=False):
 	"""Initialize logging.
 	This must be called before any logging can occur.
@@ -391,9 +408,7 @@ def initialize(shouldDoRemoteLogging=False):
 			fmt="{levelname!s} - {codepath!s} ({asctime}) - {threadName} ({thread}):\n{message}",
 			style="{"
 		)
-		if (globalVars.appArgs.secure or globalVars.appArgs.noLogging) and (not globalVars.appArgs.debugLogging and globalVars.appArgs.logLevel == 0):
-			# Don't log in secure mode.
-			# #8516: also if logging is completely turned off.
+		if _shouldDisableLogging():
 			logHandler = logging.NullHandler()
 			# There's no point in logging anything at all, since it'll go nowhere.
 			log.root.setLevel(Logger.OFF)