Improve security perception for NVDA add-ons

[Adriani90]

Discussion started in #16235. That proposal is not acceptable and not really supported by the community, so following proposal from my side. Please add your ideas as well.

following could be done to improve the security perception:

1. Publish indications in the user guide and on the corporate environment website on what users should care about when installing add-ons
2. Use a tool to detect vulnerabilities in the add-on code (i.e. as a github action in the add-on submission store with CodeqL: https://docs.github.com/en/code-security/code-scanning/introduction-to-code-scanning/about-code-scanning-with-codeql
   here is another list of alternative tools which could help in automating security checks in the add-on submission repository:
   https://owasp.org/www-community/Source_Code_Analysis_Tools
3. Release the add-on to the add-on store with a review flag to the store depending on the results of the CodeqL or any automated analysis
4. Encourage users via in process blog and mailing lists and other channels to provide reviews of the add-ons in the add-on store to build up trusted environment
5. Make a compulsory data requirement in the manifest file which requires add-on authors to describe the permissions an add-on is running on a system, and show this data for every add-on when installing as a warning dialog
   (i.e. this add-on is using an external API to process data, or this add-on saves a log file in temp\add-on-name which contains private data such as what you type on your keyboard. By installing this add-on, you allow the add-on to use the saving and processing permissions on your system. Are you sure you want to install this add-on?
   yes button; no button).

Additional details

Here is an overview of risks especially for non-malitious apps:

1. Insecure network connections
2. Files stored with insecure file permissions or in an unprotected location
3. Sensitive information written to a system log
4. Web browser vulnerabilities
5. Vulnerabilities in third-party libraries
6. Cryptographic vulnerabilities, and more.
   https://www.linkedin.com/pulse/how-ensure-mobile-app-security-workplace-prateek-panda
   I would say the first 3 points can be quite easily covered in a review without much effort, with a bit of technical knowledge.
   However, the last 3 points are tricky to discover I guess if no one has professionally doing security reviews.
   For mobile apps and browser extenstions at least, there are some tools where you can throw the code in and they generate a security report.
   Since all our add-ons are open-source, we could think about using such a tool where python code is analysed automatically to a certain extent.
   For mobile apps there is Appknox for example:
   https://www.appknox.com/
   For browser extensions there is the system center endpoint protection:
   https://software.berkeley.edu/system-center-endpoint-protection-scep-0
   and the CRxcavator where you can throw a log of the extension and it analyses it for certain security related aspects:
   https://crxcavator.io/docs.html#/
   In general it seems following behavior should people have before installing extensions of any kind. This could be rewritten as indications in the user guide and on the corporate environment website:
   • Check out the developer’s website to see if it’s a legitimate extension and not a one-off by an unvetted source. (not really relevant for us because most add-ons are on github and still we cannot make sure people upload only trusted content on it)
   • Read the description. Look for things that may be questionable, like tracking info or data sharing.
   • Check out the reviews. Look for users complaining of oddities happening, speculating on their data being taken, or for anything that strikes you as odd. (this would come into force if enough users reviewed the add-ons. This could be encouraged to do via the in process blog, motivating people on the mailing list or Masterdom or Discord to give a user review etc.)
   • Be picky. The more extensions installed, the bigger the attack surface you open up to attackers. Only pick the most useful and delete the ones you don’t need.
   • Only install through trusted sources. While not guaranteed safe, security technicians review extensions for malicious content. 
   • Review permissions. Review extension permissions closely. If an extension installed suddenly requests new permissions, be wary. If you can’t find a reason for the permission change, it’s probably better to uninstall.
   https://security.berkeley.edu/education-awareness/browser-extensions-how-vet-and-install-safely

[Adriani90]

I found a very interesting article on how to build github CI workflows / actions with codeqL and Bandit to analyse python repositories automatically as far as possible. Here is also an example repo with already built in actions to see how it works:
https://github.com/tjtharrison/demo-python-repository?tab=readme-ov-file#readme
I attach the article as a word document, it comes from here:
https://tjtharrison.medium.com/github-ci-workflows-for-python-code-repos-5e13b5538aaa
GitHub CI workflows for Python code repos.docx

[Nael-Sayegh]

Hello,
I tend to agree with your proposal @Adriani90 .
However, we should include in the documentation a list of permissions that need to be added to our manifest, so that the developer can really think about what needs to be mentioned without forgetting any details.
I am less in agreement with the idea of mentioning in the add-on store whether the add-on has been evaluated, because if the add-on is of no interest to any evaluator and since there are not many, the add-on will be displayed as potentially dangerous even though it may not be.
For example, if we take an add-on like "everything", I think very few evaluators would be motivated to evaluate it and it could remain as potentially dangerous for a long time.
However, it all depends on the mention displayed if the add-on is not reviewed.

[Adriani90]

my idea is to display "reviewed" or "not reviewed", not more than that. And there should be a documentation which explains what this status means, either in the user guide or on the corporate environment website of NV Access.
Reviewed could include:

1. Automatic check via codeqL or Bandit or other tools has been performed
   or
2. The add-on has been reviewed manually by a certified expert and a security note is added to the add-on documentation
3. Additionally, the add-on has at least one user review in the sumission store discussion for the add-on.

So the status "reviewed" is a combination of 1 and 3 or 2 and 3.

This would improve the current situation at least.

[nvdaes]

I support, for now:

4. Encourage users via in process blog and mailing lists and other channels to provide reviews of the add-ons in the add-on store to build up trusted environment. @Qchristensen may have ideas about if this is or not feasible and why.

From my side, I've just activated CodeQL from clipContentsDesigner settings, as well as ability to report vulnerabilities privately from GitHub, in addition to using my email address. I'll do this for all my add-ons in the next days.
Other points may require more ellaboration, for example:

• When an add-on is updated may change drastically, and reviews may have been performed before the update.
• • One of the testing principles is that tests can detect failures, but not absence of errors, so we should be careful avoiding to provide a false perception of security.

[Qchristensen]

Encouraging reviews is a good idea and I'm all for that and happy to do that. Re the point that an update to an add-on may completely rewrite it and change it - that is true of many things, eg apps on your phone. I'm not sure if phone app ratings are simply the average of every review that app has ever had, or if they are weighted to more recent reviews. I know in most cases where you can leave comments those are usually tagged with when they were posted so you can pay more attention to newer reviews.

[Adriani90]

Wow this is already a quick win. 😍 Noelia did you have a look at the list of tools for automatic analysis? There might be something that complements codeqL and maybe this improves the checks.Maybe we can.create a github action for the addon submission globally so that the security checks run automatically for every addon before pushing every update to the store.Von meinem iPhone gesendetAm 29.02.2024 um 21:08 schrieb Noelia Ruiz Martínez ***@***.***>: I support, for now: Encourage users via in process blog and mailing lists and other channels to provide reviews of the add-ons in the add-on store to build up trusted environment. @Qchristensen may have ideas about if this is or not feasible and why. From my side, I've just activated CodeQL from clipContentsDesigner settings, as well as ability to report vulnerabilities privately from GitHub, in addition to using my email address. I'll do this for all my add-ons in the next days. Other points may require more ellaboration, for example: When an add-on is updated may change drastically, and reviews may have been performed before the update. One of the testing principles is that tests can detect failures, but not absence of errors, so we should be careful avoiding to provide a false perception of security. —Reply to this email directly, view it on GitHub, or unsubscribe.You are receiving this because you were mentioned.Message ID: ***@***.***>

[Nael-Sayegh]

I really agree now with this proposal, the comment from @nvdaes makes me ask a question, would the security check be implemented on the add-on store repository or on the add-on repository?
If it's on the add-on repository, I will try to set it up right away like @nvdaes

[nvdaes]

I have searched for documentation to see how this can be implemented and I have done it in my personal repo. Go to repolink/settings, and there you will find security options. I have enabled QL and dependabot, and also the ability to report vulnerabilities privately on Github. A fragment shared by @Adriani90 of the article about QL contains a mention to checkout action v3, not 4, which is te latest version, so I have searched for more documentation.

[mwhapples]

This seems like a reasonable start point. I though have some questions and comments:

• What do the automated checks attempt to detect? Are the tools being used designed to catch unintended vulnerabilities and mistakes or do they detect certain types of activity which then could be compared against declarations in a manifest (eg. connecting and sending data to a server).
• Are these automated check tools designed to assist developers create correct code or are they designed to help protect against malicious code? Does this align with what is wanted for the addon store? My feeling is that malicious code may be designed to be particularly obscure in what it does and may use non-standard techniques and libraries to avoid detection but it may be of high quality from a code style point of view.
• With a manifest is there any way for this to be enforced? If the developer fails to declare they need a permission will anything stop them taking such an action? Of course action may be taken if they are found out, but if this relies upon people discovering it manually then there could be a time whilst that addon is getting onto user computers.

[Brian1Gaff]

The sort of add on that would possibly give rise to concern would be anything that involves calling another on line system, like link shorteeners and AI interfaces etc. How could one be sure that those sites don't suddenly introduce data gathering and tracking? Brian

…

-- ***@***.*** Sent via blueyonder.(Virgin media) Please address personal E-mail to:- ***@***.***, putting 'Brian Gaff' in the display name field. ----- Original Message ----- From: Nael Sayegh To: nvaccess/nvda Cc: Subscribed Sent: Thursday, February 29, 2024 7:05 PM Subject: Re: [nvaccess/nvda] Improve security perception for NVDA add-ons (Discussion #16241) Hello, I tend to agree with your proposal @Adriani90 . However, we should include in the documentation a list of permissions that need to be added to our manifest, so that the developer can really think about what needs to be mentioned without forgetting any details. I am less in agreement with the idea of mentioning in the add-on store whether the add-on has been evaluated, because if the add-on is of no interest to any evaluator and since there are not many, the add-on will be displayed as potentially dangerous even though it may not be. For example, if we take an add-on like "everything", I think very few evaluators would be motivated to evaluate it and it could remain as potentially dangerous for a long time. However, it all depends on the mention displayed if the add-on is not reviewed. — Reply to this email directly, view it on GitHub, or unsubscribe. You are receiving this because you are subscribed to this thread.Message ID: ***@***.***>

[Adriani90]

With a manifest is there any way for this to be enforced? If the developer fails to declare they need a permission will anything stop them taking such an action? Of course action may be taken if they are found out, but if this relies upon people discovering it manually then there could be a time whilst that addon is getting onto user computers.

This could be done by introducing a validation whether an add-on is transparent in its permissions or not in the compatibility checks. If the author didn't document the actions / permissions the add-on uses in its manifest, then the add-on is not transparent and is incompatible with NVDA's standards.
When an user still enables / attempts to install the incompatible add-on, there should be a warning that this might also mean the author didn't document the permissions in a transparent way.

If the permissions are documented properly in the manifest and the add-on passed all compatibility checks, the permissions could be displayed in a installation / activation dialog to inform the user.

[Adriani90]

My feeling is that malicious code may be designed to be particularly obscure in what it does and may use non-standard techniques and libraries to avoid detection but it may be of high quality from a code style point of view.

That's right. And currently there are no fully automated solution to check whether an action is sensitive or not. But here is a very interesting article on how security manager checks are done in Java, maybe this can inspire us as well:
https://wiki.sei.cmu.edu/confluence/display/java/SEC04-J.+Protect+sensitive+operations+with+security+manager+checks

Are these automated check tools designed to assist developers create correct code or are they designed to help protect against malicious code? Does this align with what is wanted for the addon store?

They are much more than that, most of them are really static analysis tools that also detect external injections of different kinds and generate recommendations or warnings. Most of them have also github integrations. See for example this tool:
https://security-code-scan.github.io/

[Adriani90]

There are also tools for permission automated checking, with permission databases in background, and some of them also seem to be able to generate a permission documentation for a code but I am not sure how good they are.
However, permission checking could be done from NVDA's side, by defining permission classes for add-ons in a pythonic way , which restrict add-ons to do certain actions in general.
But this needs first a database of actions / permissions add-ons should never have, and them make sure that these actions cannot be performed by an add-on after installation.

[seanbudd]

I think there has been some good discussion here.
I would encourage spinning up some issues

Publish indications in the user guide and on the corporate environment website on what users should care about when installing add-ons

I'm not sure this is something we can comment on other than general warning of trusting the publisher / source.

Use a tool to detect vulnerabilities in the add-on code (i.e. as a github action in the add-on submission store with CodeqL: https://docs.github.com/en/code-security/code-scanning/introduction-to-code-scanning/about-code-scanning-with-codeql
here is another list of alternative tools which could help in automating security checks in the add-on submission repository:
https://owasp.org/www-community/Source_Code_Analysis_Tools

I would encourage opening an issue in nvaccess/addon-datastore

Release the add-on to the add-on store with a review flag to the store depending on the results of the CodeqL or any automated analysis

I think from other discussion here this may not be the best UX. Code scanning is a good smoke test, something that reduces risk, but I don't think we should encourage people to rely on it.

Encourage users via in process blog and mailing lists and other channels to provide reviews of the add-ons in the add-on store to build up trusted environment

This is something we could do - @Qchristensen thoughts?

Make a compulsory data requirement in the manifest file which requires add-on authors to describe the permissions an add-on is running on a system, and show this data for every add-on when installing as a warning dialog

A restricted add-on API is something we would like to move slowly towards, but this will be a huge change to add-on infrastructure

[nvdaes]

I would encourage opening an issue in nvaccess/addon-datastore

If another person doesn't open it, I'll do. After proposals on this discussion, I enabled security tools available from gitHub for my add-ons and CodeQL shows results in pull requests. We may also create suggestions for add-on authors recommending to enable these tools.