Add ability to check issues against Virus Total #945

AdmiringWorm · 2021-02-24T14:26:59Z

This pull requests adds the ability to check if there are already virus reports available on VirusTotal for the specified download url.
If there is no report available, it will add to the comment details as such.

This code requires that a secret variable is added to the Settings page of this repository called VIRUS_TOTAL_API_KEY, and needs to be added by someone with access to a VirusTotal api key and the repository settings page.

AdmiringWorm · 2021-02-24T14:29:19Z

Example of the details comments with the information added can be seen here:

With viruses in the report: AdmiringWorm#59 (comment)
With no viruses found: AdmiringWorm#47 (comment)
With no report available: AdmiringWorm#60 (comment)

tunisiano187 · 2021-05-10T07:16:11Z

Maybe hybrid-analysis could be interesting, as VT is used by them + others ;-)
https://www.hybrid-analysis.com/docs/api/v2
max limit to 100m so no sorry

AdmiringWorm · 2021-05-10T07:22:51Z

@tunisiano187 the intention from my end isn't to upload any files, only to check and verify any existing virus scan results.

tunisiano187 · 2021-05-10T07:25:32Z

yes , Hybrid analysis do that with their own scans + Virus Total's without limit per hour ... that was the interesting point.
but i don't know if that's possible to send the hash directly to scan...

AdmiringWorm · 2021-05-10T07:29:35Z

I see, TBH, I had never heard of the website before.
Looking at it now, it does seem that it supports getting reports from a sha256 at least, but that would obviously not scan any new files.

ferdnyc · 2021-07-08T13:21:25Z

@tunisiano187

~~Maybe hybrid-analysis could be interesting, as VT is used by them + others ;-) https://www.hybrid-analysis.com/docs/api/v2~~
max limit to 100m so no sorry

It looks like those limits only apply to direct uploads, though -- I just submitted a Chocolatey package to their API directly via the download URL (swig.4.0.2.04082020.nupkg, chosen pretty much at random), and they were able to scan it from there. ~~I'm assuming the size limit wouldn't apply, going that route.~~
I assume incorrectly.

A bigger issue may be that they don't appear to support .nupkg files vey well, at current... you can see the results of the scan by looking up this hash in their API tool, if you have an API key:

33082cb5097d8535a98a80949109eb06391551b4f1eb24988635d6c7ea37f13c

They list the type as "Microsoft OOXML" (which is accurate), and the type strings as "docx" and "office" (which are not). The scan seems to be treating the package as a document, rather than an executable.

Ahhh, dangit. I just tried to point it at the URL for a larger .exe installer, and you're right -- the 100MB limit applies no matter how you submit the file to be scanned. So, doubly scratch that.

mkevenaar · 2022-06-12T08:32:40Z

Hi @AdmiringWorm I wanted to take this for a spin on your branch, but it seems that you have switched branches

AdmiringWorm · 2022-06-13T07:57:55Z

Hi @AdmiringWorm I wanted to take this for a spin on your branch, but it seems that you have switched branches

Yeah, I was working on something else and had the branch changed to the work in progress there.

I have switched back to the branch for this PR, so if you want to check it out then feel free to do so now.

gep13

Just some minor suggestions here, otherwise, this LGTM!

scripts/private/messages.ps1

JPRuskin · 2024-03-01T10:29:01Z

.vscode/settings.json

@@ -0,0 +1,3 @@
+{
+    "powershell.codeFormatting.addWhitespaceAroundPipe": true


This seems like a fine change, but may have been an unintended addition?

When this change was added it was automatically added by vscode, however as it is the default I decided to leave it in.

I've decided to get this removed again

JPRuskin · 2024-03-01T10:31:38Z

scripts/private/api/virus-total/Get-VirusTotalResults.ps1

+
+    try {
+        # next we try to serialize as hash table
+        return $inputObject | ConvertFrom-Json -AsHashtable


-AsHashtable may not be supported in PS5. We're using windows-latest in our tasks, but I think the way we're running it may take us to PowerShell rather than Pwsh. May be wrong! I don't know what we're targetting here, just noting as we normally dive into backwards compatibility.

That may also be fine, as this is being used post-IRM, which (I thought) should convert from JSON anyway, and this may just fall down to returning the object as-is. Looking at the API, though, it's... hm, fun.

-AsHashtable may not be supported in PS5.

You are right, it is not supported in PS5. We are however explicitly telling GitHub action to use Pwsh, so that is fine. Backwards compatibility is not a concern as these files aren't supposed to be ran on a users computer.

Also to mention, from what I remember when this was added the entire thing would fall over if -AsHashtable wasn't used.

JPRuskin · 2024-03-01T10:36:14Z

scripts/private/messages.ps1

@@ -19,7 +19,7 @@ class StatusCheckMessages {
    static [string]$checkingUserMarkedDownloadUrlPublic = "Checking if user have marked the download URL as being public and not bedind a paywall";
    static [string]$checkingUserProvidedDirectDownloadUrl = "Checking if the user have provided a direct download URL";
    static [string]$checkingUserProvidedSoftwareProjectUrl = "Checking if user have provided a URL to the software project";
-    static [string]$checkingUserSearchedForOpenIssues = "Checking if the user have checked for already opened issues";


I can't see associated calls for this being removed (e.g. line 69 of scripts/private/validations/Get-NewPackageValidationResult.ps1).

Good catch, not sure why it was removed, but I'll get it added back.

JPRuskin · 2024-03-01T10:39:42Z

scripts/private/messages.ps1

@@ -19,7 +19,7 @@ class StatusCheckMessages {
    static [string]$checkingUserMarkedDownloadUrlPublic = "Checking if user have marked the download URL as being public and not bedind a paywall";
    static [string]$checkingUserProvidedDirectDownloadUrl = "Checking if the user have provided a direct download URL";
    static [string]$checkingUserProvidedSoftwareProjectUrl = "Checking if user have provided a URL to the software project";
-    static [string]$checkingUserSearchedForOpenIssues = "Checking if the user have checked for already opened issues";
+    static [string]$checkingvirusTotalCheckSearchedForOpenIssues = "Checking if the user have checked for already opened issues";


Suggested change

static [string]$checkingvirusTotalCheckSearchedForOpenIssues = "Checking if the user have checked for already opened issues";

static [string]$checkingvirusTotalCheckSearchedForOpenIssues = "Checking for Virus Total issues";

Minor, phrasing to update from the previous message. The variable name is a little... cloudy. It seems like $virusTotalCheck, below, is doing this work for this. It's possible this line should be rolled back?

The suggestion here doesn't follow the same format as the other check messages (These are the messages outputted in the GitHub action, but not anywhere else).

I'll figure out something to change this to, as it is currently wrong.

EDIT: When looking again, this was incorrectly renamed. Not sure what was going on here.

JPRuskin · 2024-03-01T10:50:58Z

scripts/private/api/virus-total/Get-VirusTotalResults.ps1

@@ -0,0 +1,72 @@
+function Get-JsonResults() {


Suggested change

function Get-JsonResults() {

function Get-JsonResults {

I didn't realise you could specify param blocks both as a simple and advanced function at once! Probably worth tidying up, though.

Edit: Ah, had a play. Looks like it's only trouble if you specify parameters in both places. Interesting!

It may also be worth supporting pipelining here, but... it's here, it works. No point in changing it if it's fine, etc.

Outdated

AdmiringWorm requested review from gep13 and pauby February 24, 2021 14:27

AdmiringWorm force-pushed the virus-total branch 2 times, most recently from 7f3fd9b to 08d7df4 Compare June 3, 2022 11:28

gep13 previously requested changes Jun 30, 2022

View reviewed changes

scripts/private/messages.ps1 Outdated Show resolved Hide resolved

scripts/private/messages.ps1 Outdated Show resolved Hide resolved

scripts/private/messages.ps1 Outdated Show resolved Hide resolved

scripts/private/messages.ps1 Outdated Show resolved Hide resolved

AdmiringWorm force-pushed the virus-total branch from 08d7df4 to f220d2f Compare March 1, 2024 09:30

AdmiringWorm requested a review from gep13 March 1, 2024 09:30

AdmiringWorm force-pushed the virus-total branch from f220d2f to efb482b Compare March 1, 2024 09:40

AdmiringWorm requested review from JPRuskin and removed request for pauby March 1, 2024 10:21

JPRuskin reviewed Mar 1, 2024

View reviewed changes

AdmiringWorm added 4 commits March 1, 2024 14:29

(maint) Added ability to report on existing VirusTotal reports

730f106

(maint) Add virus total environment variables

6b1a1ad

(maint) Added special handling for file details section

80ebe78

(maint) Changed how to calculate anti-virus count

8fb1267

AdmiringWorm force-pushed the virus-total branch from efb482b to 8fb1267 Compare March 1, 2024 13:29

AdmiringWorm requested review from JPRuskin and removed request for gep13 March 1, 2024 13:29

This comment was marked as off-topic.

Sign in to view

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add ability to check issues against Virus Total #945

Add ability to check issues against Virus Total #945

AdmiringWorm commented Feb 24, 2021

AdmiringWorm commented Feb 24, 2021

tunisiano187 commented May 10, 2021 •

edited

Loading

AdmiringWorm commented May 10, 2021

tunisiano187 commented May 10, 2021 •

edited

Loading

AdmiringWorm commented May 10, 2021

ferdnyc commented Jul 8, 2021

mkevenaar commented Jun 12, 2022

AdmiringWorm commented Jun 13, 2022

gep13 left a comment

JPRuskin Mar 1, 2024

AdmiringWorm Mar 1, 2024

AdmiringWorm Mar 1, 2024

JPRuskin Mar 1, 2024

AdmiringWorm Mar 1, 2024

JPRuskin Mar 1, 2024

AdmiringWorm Mar 1, 2024

JPRuskin Mar 1, 2024

AdmiringWorm Mar 1, 2024 •

edited

Loading

JPRuskin Mar 1, 2024

JPRuskin Mar 1, 2024

This comment was marked as off-topic.

This comment was marked as off-topic.

		@@ -0,0 +1,3 @@
		{
		"powershell.codeFormatting.addWhitespaceAroundPipe": true

	static [string]$checkingvirusTotalCheckSearchedForOpenIssues = "Checking if the user have checked for already opened issues";
	static [string]$checkingvirusTotalCheckSearchedForOpenIssues = "Checking for Virus Total issues";

Add ability to check issues against Virus Total #945

Are you sure you want to change the base?

Add ability to check issues against Virus Total #945

Conversation

AdmiringWorm commented Feb 24, 2021

AdmiringWorm commented Feb 24, 2021

tunisiano187 commented May 10, 2021 • edited Loading

AdmiringWorm commented May 10, 2021

tunisiano187 commented May 10, 2021 • edited Loading

AdmiringWorm commented May 10, 2021

ferdnyc commented Jul 8, 2021

mkevenaar commented Jun 12, 2022

AdmiringWorm commented Jun 13, 2022

gep13 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

AdmiringWorm Mar 1, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

This comment was marked as off-topic.

This comment was marked as off-topic.

tunisiano187 commented May 10, 2021 •

edited

Loading

tunisiano187 commented May 10, 2021 •

edited

Loading

AdmiringWorm Mar 1, 2024 •

edited

Loading