Skip to content

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

Merged
feerrenrut merged 1 commit into
rcfrom
disableDictionariesInputGesturesSecure
Mar 16, 2022
Merged

Disable dictionary dialogs and input gesture dialog in secure mode#13489
feerrenrut merged 1 commit into
rcfrom
disableDictionariesInputGesturesSecure

Conversation

@seanbudd

@seanbudd seanbudd commented Mar 16, 2022 •
edited
Loading

Copy link
Copy Markdown
Member

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

seanbudd commented Mar 16, 2022

Copy link
Copy Markdown
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.

Sign in to view
@AppVeyorBot

This comment was marked as outdated.

Sign in to view
@seanbudd seanbudd force-pushed the disableDictionariesInputGesturesSecure branch from dda422d to bcf2175 Compare March 16, 2022 05:17
@seanbudd @feerrenrut
Disable dictionary dialogs and input gestures in secure mode
fa876b7 
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
@feerrenrut feerrenrut mentioned this pull request Mar 16, 2022
Merged
5 tasks
@lukaszgo1

This comment was marked as resolved.

Sign in to view
@seanbudd

seanbudd commented Mar 16, 2022

Copy link
Copy Markdown
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.

Sign in to view
@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

feerrenrut commented Mar 17, 2022

Copy link
Copy Markdown
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.

Reviewers

@michaelDCurran michaelDCurran Awaiting requested review from michaelDCurran michaelDCurran is a code owner automatically assigned from nvaccess/developers

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

2021.3.4

Development

Successfully merging this pull request may close these issues.

5 participants

@seanbudd @XLTechie @AppVeyorBot @lukaszgo1 @feerrenrut