From 4e3544f9f8fb32509b05300baef798d99f92bab4 Mon Sep 17 00:00:00 2001 From: Michael Curran Date: Mon, 29 Jan 2024 10:15:09 +1000 Subject: [PATCH] Merge pull request from GHSA-xg6w-23rw-39r8 * ui.browseableMessage: pass title and message into the MSHTML dialog via a Scripting.Dictionary, rather than embedding them in a single string. * Apply suggestions from code review * Update source/message.html Co-authored-by: Sean Budd * bump release version and changelog --------- Co-authored-by: Sean Budd --- source/buildVersion.py | 2 +- source/message.html | 11 ++++------- source/ui.py | 25 ++++++++++++++++++++----- user_docs/en/changes.t2t | 9 +++++++++ 4 files changed, 34 insertions(+), 13 deletions(-) diff --git a/source/buildVersion.py b/source/buildVersion.py index 6b6cd615161..ed4f8e36219 100644 --- a/source/buildVersion.py +++ b/source/buildVersion.py @@ -67,7 +67,7 @@ def formatVersionForGUI(year, major, minor): name = "NVDA" version_year = 2023 version_major = 3 -version_minor = 2 +version_minor = 3 version_build = 0 # Should not be set manually. Set in 'sconscript' provided by 'appVeyor.yml' version=_formatDevVersionString() publisher="unknown" diff --git a/source/message.html b/source/message.html index bda652d4281..0e9396315e5 100644 --- a/source/message.html +++ b/source/message.html @@ -12,13 +12,10 @@ }; function windowOnLoad() { - // #5875: string.prototype.split strips the tail when a limit is supplied, - // so use a regexp instead. - var args = window.dialogArguments.match(/^(.*?)__NVDA:split-here__([\s\S]*)$/); - // args[0] is the whole string. - if (args && args.length == 3){ - document.title= args[1]; - messageID.innerHTML= args[2]; + var args = window.dialogArguments; + if (args) { + document.title = args.item('title'); + messageID.innerHTML = args.item('message'); } } //--> diff --git a/source/ui.py b/source/ui.py index 365e61617e2..1f3aad9b604 100644 --- a/source/ui.py +++ b/source/ui.py @@ -12,9 +12,16 @@ import os import sys -from ctypes import windll, byref, POINTER, addressof +from ctypes import ( + windll, + oledll, + byref, + POINTER +) +import comtypes.client from comtypes import IUnknown from comtypes import automation +from comtypes import COMError from html import escape from logHandler import log import gui @@ -26,6 +33,7 @@ from utils.security import isRunningOnSecureDesktop + # From urlmon.h URL_MK_UNIFORM = 1 @@ -88,7 +96,6 @@ def browseableMessage(message: str, title: Optional[str] = None, isHtml: bool = @param title: The title for the message. @param isHtml: Whether the message is html """ - splitWith: str = "__NVDA:split-here__" # Unambiguous regex splitter for javascript in message.html, #14667 if isRunningOnSecureDesktop(): import wx # Late import to prevent circular dependency. wx.CallAfter(_warnBrowsableMessageNotAvailableOnSecureScreens, title) @@ -104,14 +111,22 @@ def browseableMessage(message: str, title: Optional[str] = None, isHtml: bool = title = _("NVDA Message") if not isHtml: message = f"
{escape(message)}
" - dialogString = f"{title}{splitWith}{message}" - dialogArguements = automation.VARIANT( dialogString ) + try: + d = comtypes.client.CreateObject("Scripting.Dictionary") + except COMError: + log.error("Scripting.Dictionary component unavailable") + # Translators: reported when unable to display a browsable message. + message(_("Unable to display browseable message")) + return + d.add("title", title) + d.add("message", message) + dialogArgsVar = automation.VARIANT(d) gui.mainFrame.prePopup() windll.mshtml.ShowHTMLDialogEx( gui.mainFrame.Handle , moniker , HTMLDLG_MODELESS , - addressof( dialogArguements ) , + byref(dialogArgsVar), DIALOG_OPTIONS, None ) diff --git a/user_docs/en/changes.t2t b/user_docs/en/changes.t2t index 10feba4a94b..e1cd81c612a 100644 --- a/user_docs/en/changes.t2t +++ b/user_docs/en/changes.t2t @@ -4,6 +4,15 @@ What's New in NVDA %!includeconf: ../changes.t2tconf %!includeconf: ./locale.t2tconf += 2023.3.3 = +This is a patch release to fix a security issue. +Please responsibly disclose security issues following NVDA's [security policy https://github.com/nvaccess/nvda/blob/master/security.md]. + +== Security Fixes == +- Prevents possible reflected XSS attack from crafted content to cause arbitrary code execution. +([GHSA-xg6w-23rw-39r8 https://github.com/nvaccess/nvda/security/advisories/GHSA-xg6w-23rw-39r8]) +- + = 2023.3.2 = This is a patch release to fix a security issue. The security patch in 2023.3.1 was not resolved correctly.