-
-
Notifications
You must be signed in to change notification settings - Fork 783
Sanitize browsableMessage HTML #16985
Copy link
Copy link
Closed
Labels
api-breaking-changep2https://github.com/nvaccess/nvda/blob/master/projectDocs/issues/triage.md#priorityhttps://github.com/nvaccess/nvda/blob/master/projectDocs/issues/triage.md#prioritytriagedHas been triaged, issue is waiting for implementation.Has been triaged, issue is waiting for implementation.
Milestone
Metadata
Metadata
Assignees
Labels
api-breaking-changep2https://github.com/nvaccess/nvda/blob/master/projectDocs/issues/triage.md#priorityhttps://github.com/nvaccess/nvda/blob/master/projectDocs/issues/triage.md#prioritytriagedHas been triaged, issue is waiting for implementation.Has been triaged, issue is waiting for implementation.
Type
Fields
Give feedbackNo fields configured for issues without a type.
Is your feature request related to a problem? Please describe.
ui.browsableMessagecan inject unsanitized HTML into NVDA.This is an issue if translations are passed in as unsanitized HTML.
Translations are fairly unregulated, translation strings are the only "code" included in NVDA without a direct review from NV Access or as a review as a dependency. If NVDA translations can perform RCE, we have a problem.
Considering no NVDA source code uses the
isHTMLfunctionality of this function currently, this isn't an active vector.However, if we ever start using
isHTML, it becomes an active vector, which is something we want to avoid and prevent from becoming a possibility.Describe the solution you'd like
nh3to sanitize HTML passed intobrowsableMessage.Describe alternatives you've considered
create developer warnings to ensure translations are not passed in as HTML in NVDA core
Additional context
This is an API breaking change as it will require add-on action to perform the same functionality with
browsableMessageRaised from #16369