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

Disable dictionary dialogs and input gesture dialog in secure mode #13489

Merged
merged 1 commit into from
Mar 16, 2022

Conversation

seanbudd
Copy link
Member

@seanbudd seanbudd commented Mar 16, 2022

Thank you @CyrilleB79 and @paulber19 for reporting

Link to issue number:

GitHub Advisory GHSA-wg65-7r23-h6p9: GHSA-wg65-7r23-h6p9

Summary of the issue:

The following menu items are hidden from the menu in secure mode:

  • Input gesture
  • Default dictionary
  • Voice dictionary

However it is still possible to assign gestures to the scripts allowing to open these dialogs.

Gestures being set on the system profile may "pollute" public configuration.
If unexpected gestures or speech is being replaced, a user may be unable to sign-in to Windows.

Description of how this pull request fixes the issue:

For these commands, return early without opening the dialog if NVDA runs in secure mode.

Testing strategy:

Manual test:

  • Assign 3 gestures to the scripts allowing to open the Input gesture dialog, the Default dictionary dialog and the Voice dictionary dialog.
  • Copy your config to secure screens
  • Run NVDA in secure mode and checked that these scripts do nothing.
  • Run NVDA in normal mode and checked that each of these scripts opens the expected dialog.

Known issues with pull request:

None

Change log entries:

Security fixes

- Addressed security advisory ``GHSA-wg65-7r23-h6p9``. (#13489)
  - Remove the ability to open the input gestures dialog in secure mode.
  - Remove the ability to open the default, temporary and voice dictionary dialogs in secure mode.
  -

Code Review Checklist:

  • Pull Request description:
    • description is up to date
    • change log entries
  • Testing:
    • Unit tests
    • System (end to end) tests
    • Manual testing
  • API is compatible with existing add-ons.
  • Documentation:
    • User Documentation
    • Developer / Technical Documentation
    • Context sensitive help for GUI changes
  • UX of all users considered:
    • Speech
    • Braille
    • Low Vision
    • Different web browsers
    • Localization in other languages / culture than English

@seanbudd seanbudd requested a review from a team as a code owner March 16, 2022 03:48
@seanbudd seanbudd requested review from michaelDCurran and removed request for a team March 16, 2022 03:48
@seanbudd seanbudd added this to the 2021.3.4 milestone Mar 16, 2022
@seanbudd
Copy link
Member Author

The advisory will be published after the patch release is published.

@seanbudd seanbudd force-pushed the disableDictionariesInputGesturesSecure branch from a6d400b to dda422d Compare March 16, 2022 04:20
@XLTechie

This comment was marked as resolved.

@AppVeyorBot

This comment was marked as outdated.

@seanbudd seanbudd force-pushed the disableDictionariesInputGesturesSecure branch from dda422d to bcf2175 Compare March 16, 2022 05:17
GitHub Advisory GHSA-wg65-7r23-h6p9:

Summary:
The following menu items are hidden from the menu in secure mode:

- Input gesture
- Default dictionary
- Voice dictionary
However it was still possible to assign gestures to the scripts which
open these dialogs.

Modifying speech dictionay or gestures from secure screens could result
in a denial of service.
If unexpected gestures or speech is being replaced, a user may be unable
to sign-in to Windows.

Description of change:
For these commands, return early without opening the dialog if NVDA runs
in secure mode.
@feerrenrut feerrenrut force-pushed the disableDictionariesInputGesturesSecure branch from bcf2175 to fa876b7 Compare March 16, 2022 06:56
@feerrenrut feerrenrut merged commit aaefb19 into rc Mar 16, 2022
@feerrenrut feerrenrut deleted the disableDictionariesInputGesturesSecure branch March 16, 2022 07:08
@lukaszgo1

This comment was marked as resolved.

@seanbudd
Copy link
Member Author

@lukaszgo1 @XLTechie many of the assumptions in this comment are incorrect. The security advisory, when published, should make the security implications clear.
I will look at addressing this comment once the patch release is done, and @XLTechie has had a chance to understand the issue and this PR better.

@seanbudd

This comment was marked as resolved.

@seanbudd seanbudd changed the title Disable dictionary dialogs and input gestures in secure mode Disable dictionary dialogs and input gesture dialog in secure mode Mar 17, 2022
@nvaccess nvaccess locked as resolved and limited conversation to collaborators Mar 17, 2022
@feerrenrut
Copy link
Contributor

I'm locking this conversation and removing the comments. Please wait for the security advisory to be released on Monday. This will explain the situation. If you still would like to discuss further at this point, we'll start a discussion for it.

@nvaccess nvaccess deleted a comment from CyrilleB79 Mar 17, 2022
@nvaccess nvaccess deleted a comment from XLTechie Mar 17, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants