Skip to content

Merge security fixes from 2021.3.4#13492

Merged
feerrenrut merged 7 commits into
betafrom
securityFixesFrom2021.3.4
Mar 17, 2022
Merged

Merge security fixes from 2021.3.4#13492
feerrenrut merged 7 commits into
betafrom
securityFixesFrom2021.3.4

Conversation

@feerrenrut

Copy link
Copy Markdown
Contributor

Link to issue number:

Summary of the issue:

Several security fixes have been introduced to address (soon to published) security advisories.

Description of how this pull request fixes the issue:

Note about the state of the rc branch:

  • Initially changes for 2021.3.1 were delivered to beta, subsequently it was decided to avoid complications with translations, to make changes directly on RC.
  • The changes were cherry picked from beta.
  • There were then several more changes made squashed / merged to RC
  • These changes need to be cherry picked back into the beta branch.
  • This was done in Cherry pick from RC #13185

Similarly, the security fixes already merged into the rc branch must be merged separately to beta then to master.
To ensure the same commits are in the history of the 2021.3.4 release, 2022.1 release and master, the changes were merged (not squashed) into rc, and now with this PR also into beta, then beta can be merged back to master.
Merge in:

Testing strategy:

See individual PRs

Known issues with pull request:

None

Change log entries:

Included

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 and others added 7 commits March 16, 2022 14:45
GitHub Advisory GHSA-354r-wr4v-cx28:

Summary:
With the --debug-logging NVDA command line option, it was possible to
enable debug logging in secure mode.
From a secure screen, it was possible to activate debug logging by
restarting NVDA and selecting "Restart with debug logging" in the Exit
Dialog.
This created an instance of NVDA performing debug logging from the
system profile, from a secure context.

Description of change
Prevent debug logging in secure mode.
Remove "Restart with debug logging" from the exit dialog options.
Remove "Install pending update" from the exit dialog options.
GitHub Advisory GHSA-mvc8-5rv9-w3hx

Summary:
The wx GUI inspection tool includes a python console.
If the user binds a gesture to the startWxInspectionTool script and
their config is copied to be used on logon screen, this tool could then be
opened from the logon screen.
This would allow a user to open the python console from the logon screen with
system privileges.

Description of change:
Disables opening the wx GUI inspection tool when NVDA is
running in secure mode.
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.
cherry-pick change log entries from:
2ec9bcb
@feerrenrut feerrenrut requested a review from a team as a code owner March 16, 2022 07:59
@feerrenrut feerrenrut requested review from seanbudd and removed request for a team March 16, 2022 07:59
seanbudd
seanbudd previously approved these changes Mar 16, 2022
@seanbudd seanbudd force-pushed the securityFixesFrom2021.3.4 branch from 8d2e47b to 4baaba6 Compare March 17, 2022 01:09
@seanbudd

This comment was marked as outdated.

seanbudd
seanbudd previously approved these changes Mar 17, 2022
@AppVeyorBot

This comment was marked as outdated.

@seanbudd seanbudd force-pushed the securityFixesFrom2021.3.4 branch from f89ad2b to 3ffc6f5 Compare March 17, 2022 01:56
@seanbudd

This comment was marked as outdated.

@feerrenrut

Copy link
Copy Markdown
Contributor Author

If rebased the commits will no longer match.

@feerrenrut feerrenrut marked this pull request as draft March 17, 2022 02:34
@feerrenrut feerrenrut force-pushed the securityFixesFrom2021.3.4 branch from 3ffc6f5 to 8d2e47b Compare March 17, 2022 02:47
@feerrenrut

Copy link
Copy Markdown
Contributor Author

Each of the commits:

Are on this branch and on release-2021.3.4, but they are connected via merge commits.
It's much easier to follow this with git log --graph. You can also follow it by looking at the parent SHAs for the merges.
Merge commits have two (or more) parents.

Merge commits:

@feerrenrut feerrenrut marked this pull request as ready for review March 17, 2022 03:01
@feerrenrut feerrenrut requested a review from seanbudd March 17, 2022 03:02
@feerrenrut feerrenrut merged commit 1d5cb62 into beta Mar 17, 2022
@feerrenrut feerrenrut deleted the securityFixesFrom2021.3.4 branch March 17, 2022 03:27
@nvaccessAuto nvaccessAuto added this to the 2022.2 milestone Mar 17, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants