Add theoretical security checks to every script available on the lock screen#14211
Add theoretical security checks to every script available on the lock screen#14211
Conversation
|
cc @CyrilleB79 |
See test results for failed build of commit 72ed16feb5 |
feerrenrut
left a comment
feerrenrut
left a comment
There was a problem hiding this comment.
Overall this looks good.
I didn't comment all changes, but it would be good to have an accurate log mesage when the scripts fail due to security. Additionally, for the user, it may be more friendly to supply a specific UI message.
Additionally, the consistency of the security explainer comments is good, perhaps link them all back to the set of safescripts.
See test results for failed build of commit f4d9511645 |
feerrenrut
left a comment
feerrenrut
left a comment
There was a problem hiding this comment.
I think it's best to be explicit when that the script is expected to return after checking whether the info can be exposed. Trying to prevent future regressions by making the intent explicit.
I didn't mark all the cases where return in missing. It would be safest to ensure every ui.reviewMessage(gui.blockAction.Context.WINDOWS_LOCKED.translatedMessage) should be followed by a return, and if not comment on why a different pattern is followed.
| # This script is available on the lock screen via getSafeScripts, as such | ||
| # ensure the status bar does not contain secure information | ||
| # before announcing this object | ||
| and not objectBelowLockScreenAndWindowsIsLocked(obj) |
There was a problem hiding this comment.
How does the other branch handle a locked session? E.G. foreground = api.getForegroundObject(), will foreground be None?
There was a problem hiding this comment.
I have tested reporting the title on the lock screen. "Lock Screen Window" and "Magnifier" were reported correctly from the lock screen and sign-in screen. I'm not sure how gracefully this fails.
4 participants
Link to issue number:
None
Summary of the issue:
NVDA may cache or directly access objects below the lock screen while Windows is locked.
As a result, without security checks, secure information may be leaked while Windows is locked.
"Secure objects" refer to
NVDAObjectswhich may contain secure information: i.e. objects below the lock screen while Windows is locked.In 2022.2.4, additional security checks were added to
api.setReviewCursorto prevent secure objects from being set as the review cursor.The results of
api.setReviewCursorare not acknowledged, meaning theoretically a cached secure object may be announced.Similar issues exist for other scripts available on the lock screen.
A thorough review of every script available on the lock screen is required.
There are no known exploits related to the theoretical issues that this PR attempts to solve.
Description of user facing changes
None
Description of development approach
A thorough review of every script available on the lock screen was performed.
Additional security checks were added to ensure that no secure objects or text from secure objects are cached or announced when activating a script available on the lock screen.
Testing strategy:
Test on alpha for an extensive period, as these changes are minor but widespread and hard to test manually.
Known issues with pull request:
None
Change log entries:
None required
Code Review Checklist: