Followup to PR #1918: scoop-virustotal.ps1 fixes (#1934)

pcrama · r15ch13 · commit 91a02ce03d9a · 2018-03-14T19:04:18.000+01:00
* Fix interpretation of response's status code to detect redirections * Improve documentation of virustotal subcommand - usage & configuration of virustotal_api_key - special parameter '*' to test all installed apps - make necessity of having a virustotal_api_key for --scan explicit - show that it's possible to check several packages at once * Never use virustotal_api_key to query if a package is safe The URL in the code wasn't an API end-point anyway. * Refactor logic to warn user about apps unknown to VirusTotal * Warn once when virustotal_api_key's absence prevents VirusTotal submission This is preparation for changes to come in the package submission logic. * Use API to submit download link to VirusTotal, rate limited in EAFP fashion This is a roundabout way to get the file to be scanned without having to download & upload it ourselves. Rate limiting is implemented using EAFP: if submission fails, we wait at least 60s before retrying at most once. * Color undecided VirusTotal information the same way as `dangerous' files If the scanning is still in progress, VirusTotal returns 0 malicious, 0 suspicious and 0 undetected. Err on the safe side and color this the same way as `dangerous' files. * Remove requirement to only verify installed apps The initial use case for this feature was to scan packages to avoid installing dangerous apps. Assuming they are infected, we want if possible to avoid downloading them at all. * Check dependencies with VirusTotal, too (by default) * Manually apply `Lint: PSAvoidUsingCmdletAliases' (see e1bb1e9, #2075) This is to avoid conflicts when merging lukesampson:master * Explain applist's return value transformation: drop `global' flag for each app * Move variable declarations and apps list generation to the top * Reformat code and comply to linted function names * Reduce nesting, remove hacky hash/url retrieval * Remove $global variables * Fix regression bug in Search-VirusTotal() * Remove applist() because it's irrelevant if app is installed globally
diff --git a/libexec/scoop-virustotal.ps1 b/libexec/scoop-virustotal.ps1
@@ -1,12 +1,20 @@
-# Usage: scoop virustotal <app> [options]
+# Usage: scoop virustotal [* | app1 app2 ...] [options]
 # Summary: Look for app's hash on virustotal.com
 # Help: Look for app's hash (MD5, SHA1 or SHA256) on virustotal.com
 #
+# Use a single '*' for app to check all installed apps.
+#
 # The download's hash is also a key to access VirusTotal's scan results.
 # This allows to check the safety of the files without even downloading
 # them in many cases.  If the hash is unknown to VirusTotal, the
 # download link is printed to submit it to VirusTotal.
 #
+# If you have signed up to VirusTotal's community, you have an API key
+# that this script can use to submit unknown packages for inspection
+# if you use the `--scan' flag.  Tell scoop about your API key with:
+#
+#   scoop config virustotal_api_key <your API key: 64 lower case hex digits>
+#
 # Exit codes:
 # 0 -> success
 # 1 -> problem parsing arguments
@@ -21,7 +29,10 @@
 # Options:
 #   -a, --arch <32bit|64bit>  Use the specified architecture, if the app supports it
 #   -s, --scan For packages where VirusTotal has no information, send download URL
-#              for analysis (and future retrieval)
+#              for analysis (and future retrieval).  This requires you to configure
+#              your virustotal_api_key.
+#   -n, --no-depends By default, all dependencies are checked, too.  This flag allows
+#                    to avoid it.
 
 . "$psscriptroot\..\lib\core.ps1"
 . "$psscriptroot\..\lib\help.ps1"
@@ -30,26 +41,49 @@
 . "$psscriptroot\..\lib\buckets.ps1"
 . "$psscriptroot\..\lib\json.ps1"
 . "$psscriptroot\..\lib\config.ps1"
+. "$psscriptroot\..\lib\decompress.ps1"
+. "$psscriptroot\..\lib\install.ps1"
+. "$psscriptroot\..\lib\depends.ps1"
 
 reset_aliases
 
-$opt, $apps, $err = getopt $args 'a:s' @('arch=', 'scan')
+$opt, $apps, $err = getopt $args 'a:sn' @('arch=', 'scan', 'no-depends')
 if($err) { "scoop virustotal: $err"; exit 1 }
+if(!$apps) { my_usage; exit 1 }
 $architecture = ensure_architecture ($opt.a + $opt.arch)
 
+if(is_scoop_outdated) { scoop update }
+
+$apps_param = $apps
+
+if($apps_param -eq '*') {
+    $apps = installed_apps $false
+    $apps += installed_apps $true
+}
+
+if (!$opt.n -and !$opt."no-depends") {
+    $apps = install_order $apps $architecture
+}
+
 $_ERR_UNSAFE = 2
 $_ERR_EXCEPTION = 4
 $_ERR_NO_INFO = 8
 
 $exit_code = 0
 
+# Global flag to warn only once about missing API key:
+$warned_no_api_key = $False
+
+# Global flag to explain only once about sleep between requests
+$explained_rate_limit_sleeping = $False
+
+# Requests counter to slow down requests submitted to VirusTotal as
+# script execution progresses
+$requests = 0
+
 Function Get-VirusTotalResult($hash, $app) {
     $hash = $hash.ToLower()
     $url = "https://www.virustotal.com/ui/files/$hash"
-    $api_key = get_config("virustotal_api_key")
-    if ($api_key) {
-        $url += '?apikey=' + $api_key
-    }
     $result = (new-object net.webclient).downloadstring($url)
     $stats = json_path $result '$.data.attributes.last_analysis_stats'
     $malicious = json_path $stats '$.malicious'
@@ -58,10 +92,10 @@ Function Get-VirusTotalResult($hash, $app) {
     $unsafe = [int]$malicious + [int]$suspicious
     $see_url = "see https://www.virustotal.com/#/file/$hash/detection"
     switch ($unsafe) {
-        0 {$fg = "DarkGreen"}
-        1 {$fg = "DarkYellow"}
-        2 {$fg = "Yellow"}
-        default {$fg = "Red"}
+        0 { if ($undetected -eq 0) { $fg = "Yellow" } else { $fg = "DarkGreen" } }
+        1 { $fg = "DarkYellow" }
+        2 { $fg = "Yellow" }
+        default { $fg = "Red" }
     }
     write-host -f $fg "$app`: $unsafe/$undetected, $see_url"
     if($unsafe -gt 0) {
@@ -70,23 +104,21 @@ Function Get-VirusTotalResult($hash, $app) {
     return 0
 }
 
-Function Search-VirusTotal ($h, $app) {
-    if ($h -match "(?<algo>[^:]+):(?<hash>.*)") {
-        $hash = $matches["hash"]
-        if ($matches["algo"] -match "(md5|sha1|sha256)") {
+Function Search-VirusTotal ($hash, $app) {
+    if ($hash -match '(?<algo>[^:]+):(?<hash>.*)') {
+        $hash = $matches['hash']
+        if ($matches['algo'] -match '(md5|sha1|sha256)') {
             return Get-VirusTotalResult $hash $app
-        }
-        else {
-            warn("$app`: Unsupported hash $($matches['algo']). VirusTotal needs md5, sha1 or sha256.")
+        } else {
+            warn "$app`: Unsupported hash $($matches['algo']). VirusTotal needs md5, sha1 or sha256."
             return $_ERR_NO_INFO
         }
     }
-    else {
-        return Get-VirusTotalResult $h $app
-    }
+
+    return Get-VirusTotalResult $hash $app
 }
 
-Function Get-RedirectedUrl {
+Function Submit-RedirectedUrl {
     # Follow up to one level of HTTP redirection
     #
     # Copied from http://www.powershellmagazine.com/2013/01/29/pstip-retrieve-a-redirected-url/
@@ -99,7 +131,7 @@ Function Get-RedirectedUrl {
     $request = [System.Net.WebRequest]::Create($url)
     $request.AllowAutoRedirect=$false
     $response=$request.GetResponse()
-    if ($response.StatusCode -eq "Found") {
+    if (([int]$response.StatusCode -ge 300) -and ([int]$response.StatusCode -lt 400)) {
         $redir = $response.GetResponseHeader("Location")
     }
     else {
@@ -109,109 +141,96 @@ Function Get-RedirectedUrl {
     return $redir
 }
 
-Function Submit-ToVirusTotal ($url, $app, $do_scan) {
-    if ($do_scan) {
-        try {
-            # Follow redirections (for e.g. sourceforge URLs) because
-            # VirusTotal analyzes only "direct" download links
-            $url = $url.Split("#").GetValue(0)
-            $new_redir = $url
-            do {
-                $orig_redir = $new_redir
-                $new_redir = Get-RedirectedUrl $orig_redir
-            } while ($orig_redir -ne $new_redir)
-            $uri = "https://www.virustotal.com/ui/urls?url=$new_redir"
-            $api_key = get_config("virustotal_api_key")
-            if ($api_key) {
-                $url += '&apikey=' + $api_key
-            }
-            Invoke-RestMethod -Method POST -Uri $uri | Out-Null
-            $submitted = $True
-        } catch [Exception] {
-            warn("$app`: VirusTotal submission failed`: $($_.Exception.Message)")
-            $submitted = $False
-            return
-        }
-    }
-    else {
-        $submitted = $False
-    }
-    if ($submitted) {
-        warn("$app`: not found`: submitted $url")
+# Submit-ToVirusTotal
+# - $url: where file to check can be downloaded
+# - $app: Name of the application (used for reporting)
+# - $do_scan: [boolean flag] whether to actually submit to VirusTotal
+#             This is a parameter instead of conditionnally calling
+#             the function to consolidate the warning message
+# - $retrying: [boolean] Optional, for internal use to retry
+#              submitting the file after a delay if the rate limit is
+#              exceeded, without risking an infinite loop (as stack
+#              overflow) if the submission keeps failing.
+Function Submit-ToVirusTotal ($url, $app, $do_scan, $retrying=$False) {
+    $api_key = get_config("virustotal_api_key")
+    if ($do_scan -and !$api_key -and !$warned_no_api_key) {
+        $warned_no_api_key = $true
+        info "Submitting unknown apps needs a VirusTotal API key.  " +
+             "Set it up with`n`tscoop config virustotal_api_key <API key>"
+
     }
-    else {
-        warn("$app`: not found`: manually submit $url")
+    if (!$do_scan -or !$api_key) {
+        warn "$app`: not found`: manually submit $url"
+        return
     }
-}
-
-if(!$apps) {
-    my_usage; exit 1
-}
-
-if(is_scoop_outdated) {
-    scoop update
-}
 
-$apps_param = $apps
+    try {
+        # Follow redirections (for e.g. sourceforge URLs) because
+        # VirusTotal analyzes only "direct" download links
+        $url = $url.Split("#").GetValue(0)
+        $new_redir = $url
+        do {
+            $orig_redir = $new_redir
+            $new_redir = Submit-RedirectedUrl $orig_redir
+        } while ($orig_redir -ne $new_redir)
+        $requests += 1
+        $result = Invoke-WebRequest -Uri "https://www.virustotal.com/vtapi/v2/url/scan" -Body @{apikey=$api_key;url=$new_redir} -Method Post -UseBasicParsing
+        $submitted = $result.StatusCode -eq 200
+        if ($submitted) {
+            warn "$app`: not found`: submitted $url"
+            return
+        }
 
-if($apps_param -eq '*') {
-    $apps = applist (installed_apps $false) $false
-} else {
-    $apps = ensure_all_installed $apps_param
+        # EAFP: submission failed -> sleep, then retry
+        if (!$retrying) {
+            if (!$explained_rate_limit_sleeping) {
+                $explained_rate_limit_sleeping = $True
+                info "Sleeping 60+ seconds between requests due to VirusTotal's 4/min limit"
+            }
+            Start-Sleep -s (60 + $requests)
+            Submit-ToVirusTotal $new_redir $app $do_scan $True
+        } else {
+            warn "$app`: VirusTotal submission of $url failed`:`n" +
+                    "`tAPI returned $($result.StatusCode) after retrying"
+        }
+    } catch [Exception] {
+        warn "$app`: VirusTotal submission failed`: $($_.Exception.Message)"
+        return
+    }
 }
 
-$requests = 0
-
 $apps | ForEach-Object {
-    ($app, $global) = $_
+    $app = $_
+    # write-host $app
     $manifest, $bucket = find_manifest $app
     if(!$manifest) {
         $exit_code = $exit_code -bor $_ERR_NO_INFO
-        warn("$app`: manifest not found")
-        return
-    }
-
-    $hash = hash $manifest $architecture
-    if (!$hash) {
-        $exit_code = $exit_code -bor $_ERR_NO_INFO
-        warn("$app`: hash not found in manifest")
+        warn "$app`: manifest not found"
         return
     }
 
-    $url = url $manifest $architecture
+    $urls = url $manifest $architecture
+    $urls | ForEach-Object {
+        $url = $_
+        $hash = hash_for_url $manifest $url $architecture
 
-    # Hacky way to see if $hash is an array (i.e. there was a list of
-    # hashes in the manifest) or a string (i.e. there was 1! hash in
-    # the manifest).
-    if ($hash[0].Length -eq 1) {
-        # Wrap download URL in array to traverse it in lockstep with
-        # the loop over the hash.
-        $url = @($url)
-    }
-
-    $hash | ForEach-Object { $i = 0 } {
-        $requests += 1
-        if ($requests -eq 5) {
-            info("Sleeping 60+ seconds between requests due to VirusTotal's 4/min limit")
-        }
-        if ($requests -gt 4) {
-            Start-Sleep -s (50 + ($requests * 2))
-        }
         try {
-            $exit_code = $exit_code -bor (Search-VirusTotal $_ $app)
+            if($hash) {
+                $exit_code = $exit_code -bor (Search-VirusTotal $hash $app)
+            } else {
+                warn "$app`: Can't find hash for $url"
+            }
         } catch [Exception] {
             $exit_code = $exit_code -bor $_ERR_EXCEPTION
             if ($_.Exception.Message -like "*(404)*") {
-                Submit-ToVirusTotal $url[$i] $app ($opt.scan -or $opt.s)
-            }
-            else {
+                Submit-ToVirusTotal $url $app ($opt.scan -or $opt.s)
+            } else {
                 if ($_.Exception.Message -match "\(204|429\)") {
-                    abort "$app`: VirusTotal request failed`: $($_.Exception.Message)" $exit_code
+                    abort "$app`: VirusTotal request failed`: $($_.Exception.Message)", $exit_code
                 }
-                warn("$app`: VirusTotal request failed`: $($_.Exception.Message)")
+                warn "$app`: VirusTotal request failed`: $($_.Exception.Message)"
             }
         }
-        $i = $i + 1
     }
 }
 
