search

nvaccess / addon-datastore

Where add-ons are submitted to the NVDA Add-on Store
https://www.nvaccess.org/
Other
20 stars 14 forks source link

Use the VirusTotal API to scan submitted add-ons for malicious content #3246

Closed seanbudd closed 5 months ago

seanbudd commented 7 months ago

The scanning implemented in #2660 generally covers cases where an add-on author may not be aware of security risks of an add-on. In general, CodeQL scanning is more designed around finding security issues caused by accident/ignorance, rather than maliciously designed code. A maliciously constructed add-on could be built and potentially pass these checks. Scanning with VirusTotal will further catch dangerous add-ons i.e. add-ons bundled with known malware.

https://docs.virustotal.com/reference/overview

nvdaes commented 7 months ago

I've found this GitHub Action to scan release assets with Virus Total.

nvdaes commented 7 months ago

Also, local files can be scanned, so that when the add-on is downloaded after crating the submission issue, it maybe used. I don't have an API key, but if it's free I'll test in my fork of the store after finishing the PR about CodeQL.

nvdaes commented 7 months ago

I think that we should analize .exe files after unzipping the add-on, and, if Virus Total analizes them, we can post link to analysis in the corresponding issue. Since we don't know how many files we will find in add-ons, I think that creating a script that checks for each json file produced in the analysis will be expensive and unpredictable, and perhaps for this reason the mentioned GitHub Action reports just filenames and links to Virus Total reports. Luckily, we don't have many add-ons with .exe files. If someone submitting such kind of add-ons is considered a trusted author, a list of trusted add-ons maybe created for updates.

CyrilleB79 commented 7 months ago

Is there only the possibility to analyse .exe files? Not other files such as .dll or .pyd?

Also, we can imagine that an add-on embeds a malicious executable code in a .exe renamed to .py, .pycle, .dat (or whatever extension). Then at runtime it may rename the file to .exe and execute it.

Is there any reason to restrict scanning to .exe?

nvdaes commented 7 months ago

Cyrille wrote:

Is there only the possibility to analyse .exe files? Not other files such as .dll or .pyd?

We can try to analize whatever file we went, even the .nvda-addon itself.

Also, we can imagine that an add-on embeds a malicious executable code in a .exe renamed to .py, .pycle, .dat (or whatever extension). Then at runtime it may rename the file to .exe and execute it.

Oh, it's true. I have also thought that non bundled files maybe downloaded.

Is there any reason to restrict scanning to .exe?

No. I've tried to analize emoticons add-on and of course it's not detected as malware. So I have thought that maybe better to analize unzipped files. I've tested today for the first time. I have been able to upload the emoticons add-on from the web interface, not using my recently created API key. I think that for testing I'll fork the used GitHub Action, who includes a workflow for testing the action itself, to discard problems when I pasted my API key in a repository secret. Ideally we should be able to analize single add-ons. Another problem is that I don't know how to simulate a malicious add-on to test what happens in those cases.

seanbudd commented 7 months ago

VirusTotal should be able to scan a whole add-on as a ZIP file if the add-on is uploaded with the extension changed to .zip

XLTechie commented 7 months ago

@nvdaes For testing, you may use these:

https://help.eset.com/emsx/7.3/en-US/antivirus_test.html

https://www.computerhope.com/issues/ch001386.htm

There are many more such things for testing positive virus scans out there.

nvdaes commented 7 months ago

Thanks @xltechie. I"ll use this for testing in the next days.

nvdaes commented 7 months ago

For reference. I'll try to use the Virus Total command line:

https://github.com/VirusTotal/vt-cli

nvdaes commented 7 months ago

I've got the following artifact analyzing clipContentsDesigner 31.'.0.0 add-on. I think that we can show the workflow like done in codeQl analysis, if we find failures, malicious or suspicious positive numbers in stats. Artifact (with the default retention days https://github.com/nvdaes/addon-datastore/actions/runs/8739900542/artifacts/1426240194

nvdaes commented 7 months ago

Aother artifact including analysis of a test provided by @XLTechie , for some reason empty. Note: I used *.json in the path artifact, so discussions and submitters.json are included.

https://github.com/nvdaes/addon-datastore/actions/runs/8743424504/artifacts/1427157401

In this case, the results key in the vt.json file is not empty. I still think that we should use the stats key

nvdaes commented 7 months ago

I'm seeing that analysis for clipContentsDesigner 31.0.0 add-on, which doesn't include malware, is not consistent between checks. For example,in one of mytest the status is quewed (maybe that VirusTotaldidn't finish the analysis). Also once I saw 3 failures (I suppose that this means that 3antivirus failed to analize the add-on). And reports are updated from time to time. So I suppose that we should block add-ons whose analysis result includes a report of at least one malicious result, and also we may add the virusTotalAnalysisURL, similar to the review URL, so that people can easily find the last report:

https://docs.virustotal.com/docs/most-recent-report

seanbudd commented 6 months ago

before closing this, we'd like to run a scan of all submitted add-ons with virus total

nvdaes commented 6 months ago

Sean wrote:

before closing this, we'd like to run a scan of all submitted add-ons with virus total

I'm working on this. I've created a workflow to download each add-on and submit it to virusTotal with my API key. Now they should be analyzed. At least one add-on hasn't been submitted: https://github.com/nvdaes/addon-datastore/actions/runs/9223105164/job/25375629531

josephsl commented 6 months ago

Hi,

Looks like there is an issue with the modified YML file with the workflow output for Windows App Essentials 24.06 submission saying:

Invalid workflow file: .github/workflows/sendJsonFile.yml#L95 error parsing called workflow ".github/workflows/sendJsonFile.yml" -> "./.github/workflows/checkAndSubmitAddonMetadata.yml" (source branch with sha:313f6d05ae992ecb7fcbe8fbf0373266098a02ca) : You have an error in your yaml syntax on line 315

This could be the reason why add-on submissions from yesterday are not being processed.

Thanks.

nvdaes commented 6 months ago

This has been fixed, but seems to be problems with the VirusTotal CLI or API at this moment. This has worked well, but seems that VirusTotal CLI is not generating well formed json files. Also, I tried to submit all add-ons available on the store to VirusTotal, to be analyzed after they have been scanned. Most of them have been submitted, but some seems to have invalid URLs. Here's the log file of GitHub Actions. Searching the word "Error" we can see not submitted add-ons (the json filename and the add-on download URL). logs_24152049556.zip

seanbudd commented 6 months ago

@nvdaes - can you please open a new PR to nvaccess/addon-datastore-staging? I've reverted previous PRs

nvdaes commented 5 months ago

Thanks @seanbudd . An add-on has been submitted afterVirusTotal support has beeen added. When the add-on submission is merged, I'll inform about the VirusTotal support on the add-ons mailing list. For reference, the submitted add-on belongs to @jpavonabian