Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Sanitize browsableMessage HTML #16985

Open
seanbudd opened this issue Aug 9, 2024 · 0 comments
Open

Sanitize browsableMessage HTML #16985

seanbudd opened this issue Aug 9, 2024 · 0 comments
Labels
api-breaking-change p2 https://github.com/nvaccess/nvda/blob/master/projectDocs/issues/triage.md#priority triaged Has been triaged, issue is waiting for implementation.
Milestone

Comments

@seanbudd
Copy link
Member

seanbudd commented Aug 9, 2024

Is your feature request related to a problem? Please describe.

ui.browsableMessage can 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 isHTML functionality 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

  • Use nh3 to sanitize HTML passed into browsableMessage.
  • Create a suitable rules allow-list for nh3
  • make the sanitization rules for this as part of our public API so add-ons can adjust them as necessary

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 browsableMessage

Raised from #16369

@seanbudd seanbudd added this to the 2025.1 milestone Aug 9, 2024
@seanbudd seanbudd added p2 https://github.com/nvaccess/nvda/blob/master/projectDocs/issues/triage.md#priority triaged Has been triaged, issue is waiting for implementation. labels Aug 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api-breaking-change p2 https://github.com/nvaccess/nvda/blob/master/projectDocs/issues/triage.md#priority triaged Has been triaged, issue is waiting for implementation.
Projects
None yet
Development

No branches or pull requests

1 participant