diff --git a/powershell/ql/lib/semmle/code/powershell/frameworks/System.IO.model.yml b/powershell/ql/lib/semmle/code/powershell/frameworks/System.IO.model.yml index 85dfd5c90802..96f035099a16 100644 --- a/powershell/ql/lib/semmle/code/powershell/frameworks/System.IO.model.yml +++ b/powershell/ql/lib/semmle/code/powershell/frameworks/System.IO.model.yml @@ -36,4 +36,13 @@ extensions: pack: microsoft/powershell-all extensible: summaryModel data: - - ["system.io.path!", "Method[getfullpath]", "Argument[0]", "ReturnValue", "taint"] \ No newline at end of file + - ["system.io.path!", "Method[getfullpath]", "Argument[0]", "ReturnValue", "taint"] + - ["system.io.file!", "Method[readallbytes]", "Argument[0]", "ReturnValue", "taint"] + - ["system.io.file!", "Method[readallbytesasync]", "Argument[0]", "ReturnValue", "taint"] + - ["system.io.file!", "Method[appendtext]", "Argument[0]", "ReturnValue", "taint"] + - ["system.io.file!", "Method[createtext]", "Argument[0]", "ReturnValue", "taint"] + - ["system.io.file!", "Method[readalllines]", "Argument[0]", "ReturnValue", "taint"] + - ["system.io.file!", "Method[readalllinesasync]", "Argument[0]", "ReturnValue", "taint"] + - ["system.io.file!", "Method[readalltext]", "Argument[0]", "ReturnValue", "taint"] + - ["system.io.file!", "Method[readalltextasync]", "Argument[0]", "ReturnValue", "taint"] + - ["system.io.fileinfo", "Method[createtext]", "Argument[0]", "ReturnValue", "taint"] \ No newline at end of file diff --git a/powershell/ql/src/queries/security/cwe-494/DownloadWithoutIntegrityCheck.qhelp b/powershell/ql/src/queries/security/cwe-494/DownloadWithoutIntegrityCheck.qhelp new file mode 100644 index 000000000000..9af562220470 --- /dev/null +++ b/powershell/ql/src/queries/security/cwe-494/DownloadWithoutIntegrityCheck.qhelp @@ -0,0 +1,62 @@ + + + + + +

+ Downloading an artifact (such as an executable, installer, archive, or script) and then + using it without verifying its integrity allows an attacker who can tamper with the artifact + to execute arbitrary code on the machine. Even when the artifact is retrieved from a trusted + source such as GitHub over HTTPS, the contents can still be replaced through a compromised + account, a malicious release asset, a poisoned cache or mirror, or a man-in-the-middle attack. +

+

+ Without an integrity check, there is no guarantee that the bytes that were downloaded are the + bytes that were intended, so the downloaded artifact must not be trusted. +

+
+ + +

+ Verify the integrity of every downloaded artifact before using it. Compute a cryptographic hash + of the downloaded file with Get-FileHash and compare it against a known-good hash + that is obtained through a trusted, out-of-band channel (for example, the signed release notes + or a published checksum file). Only use the artifact when the computed hash matches the expected + value; otherwise, discard it and fail. Where available, prefer verifying a digital signature of + the artifact in addition to, or instead of, a plain hash comparison. +

+
+ + +

+ In the following example, an executable is downloaded from GitHub and then run directly. Because + the artifact is never verified, a tampered download is executed without detection. +

+ + +

+ In the following example, the downloaded artifact's SHA-256 hash is compared against the expected + value before it is used. The artifact is removed and an error is raised if the integrity check + fails, so a tampered download is never executed. +

+ +
+ + +
  • + Common Weakness Enumeration: + CWE-494: Download of Code Without Integrity Check. +
  • +
  • + Common Weakness Enumeration: + CWE-829: Inclusion of Functionality from Untrusted Control Sphere. +
  • +
  • + Microsoft Learn: + Get-FileHash. +
  • +
    + +
    diff --git a/powershell/ql/src/queries/security/cwe-494/DownloadWithoutIntegrityCheck.ql b/powershell/ql/src/queries/security/cwe-494/DownloadWithoutIntegrityCheck.ql new file mode 100644 index 000000000000..f9c1d98aeab4 --- /dev/null +++ b/powershell/ql/src/queries/security/cwe-494/DownloadWithoutIntegrityCheck.ql @@ -0,0 +1,145 @@ +/** + * @name Unvalidated Artifact Download + * @description Download of artifact without integrity check. + * @kind problem + * @problem.severity warning + * @security-severity 7.5 + * @precision high + * @id powershell/download-without-integrity-check + * @tags security + * external/cwe/cwe-494 + * external/cwe/cwe-829 + */ + +import powershell +import semmle.code.powershell.dataflow.DataFlow +import semmle.code.powershell.dataflow.TaintTracking + +/** Holds if `s` looks like a URL pointing at a trusted artifact host. */ +bindingset[s] +predicate isTrustedArtifactHost(string s) { + s.matches([ + "%github%", + "%gitlab%", + "%bitbucket%", + "%sourceforge%", + "%powershellgallery%", + "%nuget%", + "%npmjs%", + "%pypi%", + "%repo1.maven%", + "%repo.maven.apache%", + "%blob.core.windows%", + "%amazonaws%", + "%googleapis%", + "%azure%", + "%visualstudio%", + "%jfrog%", + "%artifactory%" + ]) +} + +/** A data-flow node that is tainted by a string constant looking like an artifact URL. */ +class ArtifactUrl extends DataFlow::Node { + ArtifactUrl() { + exists(DataFlow::Node source, string s | + TaintTracking::localTaint(source, this) and + s = + [ + source.asExpr().getValue().asString(), + source.asExpr().getExpr().(ExpandableStringExpr).getUnexpandedValue() + ].toLowerCase() and + isTrustedArtifactHost(s) and + // Exclude API metadata endpoints (e.g. api.github.com/.../releases/latest), + // which return JSON metadata rather than a downloadable artifact. + not s.matches(["%api.github.com%", "%api.bitbucket.org%"]) + ) + } +} + +/** + * A call that downloads an artifact from a trusted host. + * + * This covers cmdlets and their aliases (`Invoke-WebRequest`/`iwr`, + * `Invoke-RestMethod`/`irm`, `Start-BitsTransfer`), native download tools + * (`curl`, `wget`, `azcopy`, `aria2c`) and the .NET `WebClient`/`HttpClient` + * download methods. The URL may be passed as a named argument (e.g. `-Uri`, + * `-Source`), positionally, or as a method argument. + */ +class DownloadCall extends DataFlow::CallNode { + ArtifactUrl url; + + DownloadCall() { + this.getAName() = + [ + // cmdlets and aliases + "Invoke-WebRequest", "iwr", "Invoke-RestMethod", "irm", "Start-BitsTransfer", + // native command-line download tools + "curl", "curl.exe", "wget", "wget.exe", "azcopy", "azcopy.exe", "aria2c", "aria2c.exe", + // .NET WebClient / HttpClient download methods + "DownloadFile", "DownloadFileAsync", "DownloadFileTaskAsync", "DownloadData", + "DownloadDataAsync", "DownloadDataTaskAsync", "DownloadString", "DownloadStringAsync", + "GetByteArrayAsync", "GetStreamAsync" + ] and + url = this.getAnArgument() + } + + /** + * Gets the argument that names the file the artifact is written to, if any. + * Downloads that consume the response inline (e.g. `irm ... | iex`) have no + * such argument. + */ + DataFlow::Node getOutFileArg() { + result = + this.getNamedArgument([ + "outfile", "destination", "outputfile", "outpath", "literalpath", "path", "o" + ]) + or + // WebClient.DownloadFile(url, destinationFile): the destination is the 2nd argument. + this.getAName() = ["DownloadFile", "DownloadFileAsync", "DownloadFileTaskAsync"] and + result = this.getArgument(1) + } +} + +/** + * A call that verifies the integrity of a file, by computing/comparing a hash + * or by checking a signature. + */ +class IntegrityCheck extends DataFlow::CallNode { + IntegrityCheck() { + this.getAName() = + [ + "Get-FileHash", "gfh", // hash a file + "certutil", "certutil.exe", // certutil -hashfile SHA256 + "ComputeHash", // [SHA256]::Create().ComputeHash(...) + "Get-AuthenticodeSignature", "Test-FileCatalog", // signature / catalog checks + "cosign", "cosign.exe", "gpg", "gpg.exe" // external signature verification + ] + } + + /** Gets an argument referring to the file whose integrity is being checked. */ + DataFlow::Node getFile() { result = this.getAnArgument() } +} + +module Conf implements DataFlow::ConfigSig { + predicate isSource(DataFlow::Node source) { source = any(DownloadCall c).getOutFileArg() } + + predicate isSink(DataFlow::Node sink) { sink = any(IntegrityCheck c).getFile() } +} + +module Flow = TaintTracking::Global; + +/** Holds if the downloaded file `out` flows to an integrity check. */ +predicate isVerified(DataFlow::Node out) { + exists(Flow::PathNode source | + source.getNode() = out and + source.isSource() + ) +} + +from DownloadCall call, DataFlow::Node out +where + out = call.getOutFileArg() and + not isVerified(out) +select call, + "This downloads an artifact without verifying its integrity (e.g. a hash or signature check)." diff --git a/powershell/ql/src/queries/security/cwe-494/examples/powershell/DownloadWithoutIntegrityCheckBad.ps1 b/powershell/ql/src/queries/security/cwe-494/examples/powershell/DownloadWithoutIntegrityCheckBad.ps1 new file mode 100644 index 000000000000..d406ad64c79d --- /dev/null +++ b/powershell/ql/src/queries/security/cwe-494/examples/powershell/DownloadWithoutIntegrityCheckBad.ps1 @@ -0,0 +1,10 @@ +# BAD: An artifact is downloaded from GitHub and then executed without +# verifying that its contents match a known-good hash. If the release asset is +# replaced (for example, through a compromised account, a tampered mirror, or a +# man-in-the-middle attack), arbitrary code runs on the machine. +$uri = "https://github.com/example/project/releases/download/v1.2.3/installer.exe" +$outFile = Join-Path $env:TEMP "installer.exe" + +Invoke-WebRequest -Uri $uri -OutFile $outFile + +Start-Process -FilePath $outFile -Wait diff --git a/powershell/ql/src/queries/security/cwe-494/examples/powershell/DownloadWithoutIntegrityCheckGood.ps1 b/powershell/ql/src/queries/security/cwe-494/examples/powershell/DownloadWithoutIntegrityCheckGood.ps1 new file mode 100644 index 000000000000..fa7646ec2815 --- /dev/null +++ b/powershell/ql/src/queries/security/cwe-494/examples/powershell/DownloadWithoutIntegrityCheckGood.ps1 @@ -0,0 +1,18 @@ +# GOOD: The artifact is downloaded from GitHub and its SHA-256 hash is compared +# against the expected value (published by the author through a trusted channel, +# such as the signed release notes) before it is used. The artifact is only +# executed once its integrity has been confirmed. +$uri = "https://github.com/example/project/releases/download/v1.2.3/installer.exe" +$outFile = Join-Path $env:TEMP "installer.exe" +$expectedHash = "8c954cd9b6c8f8180a05f1de66ee555f0c44b4c6738120785d827521bb8d54df" + +Invoke-WebRequest -Uri $uri -OutFile $outFile + +$actualHash = (Get-FileHash -Path $outFile -Algorithm SHA256).Hash + +if ($actualHash -ne $expectedHash) { + Remove-Item -Path $outFile -Force + throw "Integrity check failed: '$outFile' does not match the expected hash." +} + +Start-Process -FilePath $outFile -Wait diff --git a/powershell/ql/test/query-tests/security/cwe-494/DownloadWithoutIntegrityCheck.expected b/powershell/ql/test/query-tests/security/cwe-494/DownloadWithoutIntegrityCheck.expected new file mode 100644 index 000000000000..e44db9cf3629 --- /dev/null +++ b/powershell/ql/test/query-tests/security/cwe-494/DownloadWithoutIntegrityCheck.expected @@ -0,0 +1,7 @@ +| test.ps1:22:3:22:47 | Call to invoke-webrequest | This downloads an artifact without verifying its integrity (e.g. a hash or signature check). | +| test.ps1:40:3:40:103 | Call to invoke-webrequest | This downloads an artifact without verifying its integrity (e.g. a hash or signature check). | +| test.ps1:45:3:45:89 | Call to iwr | This downloads an artifact without verifying its integrity (e.g. a hash or signature check). | +| test.ps1:66:3:66:95 | Call to downloadfile | This downloads an artifact without verifying its integrity (e.g. a hash or signature check). | +| test.ps1:71:3:71:118 | Call to downloadfile | This downloads an artifact without verifying its integrity (e.g. a hash or signature check). | +| test.ps1:76:3:76:116 | Call to start-bitstransfer | This downloads an artifact without verifying its integrity (e.g. a hash or signature check). | +| test.ps1:81:3:81:91 | Call to curl | This downloads an artifact without verifying its integrity (e.g. a hash or signature check). | diff --git a/powershell/ql/test/query-tests/security/cwe-494/DownloadWithoutIntegrityCheck.qlref b/powershell/ql/test/query-tests/security/cwe-494/DownloadWithoutIntegrityCheck.qlref new file mode 100644 index 000000000000..e967e6675f42 --- /dev/null +++ b/powershell/ql/test/query-tests/security/cwe-494/DownloadWithoutIntegrityCheck.qlref @@ -0,0 +1,2 @@ +query: queries/security/cwe-494/DownloadWithoutIntegrityCheck.ql +postprocess: utils/test/InlineExpectationsTestQuery.ql diff --git a/powershell/ql/test/query-tests/security/cwe-494/test.ps1 b/powershell/ql/test/query-tests/security/cwe-494/test.ps1 new file mode 100644 index 000000000000..df443d37aeb0 --- /dev/null +++ b/powershell/ql/test/query-tests/security/cwe-494/test.ps1 @@ -0,0 +1,159 @@ +# CWE-494 - Download of a GitHub release artifact without an integrity check. +# +# Each function below is one download idiom. It documents, as inline +# expectations, which idioms the query catches and which it still misses: +# +# # $ Alert -> the query reports here (expected true positive) +# # $ MISSING: Alert -> the query SHOULD report here but does not yet +# (known false negative) +# # $ SPURIOUS: Alert -> the query reports here but should not +# (known false positive) +# +# When the query is improved to close a gap, flip MISSING -> Alert (or remove +# SPURIOUS) and re-accept the .expected file. + +# --------------------------------------------------------------------------- +# COVERED today: a recognised cmdlet with a NAMED -Uri and a NAMED -OutFile. +# --------------------------------------------------------------------------- + +function Iwr_NamedUri_OutFile($outFile) { + # BAD - baseline; download from github with no hash check. + $uri = "https://github.com/example/project/releases/download/v1.2.3/installer.exe" + Invoke-WebRequest -Uri $uri -OutFile $outFile # $ Alert +} + +function Iwr_NamedUri_OutFile_Hashed($outFile) { + # GOOD - Get-FileHash barrier on the downloaded file (correctly NOT flagged). + $uri = "https://github.com/example/project/releases/download/v1.2.3/installer.exe" + $expected = "8c954cd9b6c8f8180a05f1de66ee555f0c44b4c6738120785d827521bb8d54df" + Invoke-WebRequest -Uri $uri -OutFile $outFile + $actual = (Get-FileHash -Path $outFile -Algorithm SHA256).Hash + if ($actual -ne $expected) { Remove-Item $outFile -Force; throw "hash mismatch" } +} + +# --------------------------------------------------------------------------- +# MISSING: argument-shape variants of Invoke-WebRequest / Invoke-RestMethod. +# --------------------------------------------------------------------------- + +function Iwr_PositionalUri($outFile) { + # BAD - URL passed positionally, not as -Uri. + Invoke-WebRequest "https://github.com/example/project/releases/download/v1/app.exe" -OutFile $outFile # $ Alert +} + +function Iwr_Alias_Positional($outFile) { + # BAD - iwr alias + positional URL. + iwr "https://github.com/example/project/releases/download/v1/app.zip" -OutFile $outFile # $ Alert +} + +function Iwr_NoOutFile_PipedToIex() { + # BAD - no -OutFile; content executed inline. + $r = Invoke-WebRequest -Uri "https://github.com/example/project/releases/download/v1/bootstrap.ps1" # $ MISSING: Alert + Invoke-Expression $r.Content +} + +function Irm_NoOutFile() { + # BAD - Invoke-RestMethod returns content, no -OutFile. + irm "https://github.com/example/project/releases/download/v1/install.ps1" | Invoke-Expression # $ MISSING: Alert +} + +# --------------------------------------------------------------------------- +# MISSING: download sinks that are not in the cmdlet name list at all. +# --------------------------------------------------------------------------- + +function WebClient_DownloadFile($outFile) { + # BAD - System.Net.WebClient.DownloadFile. + $wc = New-Object System.Net.WebClient + $wc.DownloadFile("https://github.com/example/project/releases/download/v1/app.exe", $outFile) # $ Alert +} + +function WebClient_Inline($outFile) { + # BAD - inline (New-Object Net.WebClient).DownloadFile(...). + (New-Object Net.WebClient).DownloadFile("https://github.com/example/project/releases/download/v1/app.exe", $outFile) # $ Alert +} + +function Bits_Transfer($outFile) { + # BAD - Start-BitsTransfer. + Start-BitsTransfer -Source "https://github.com/example/project/releases/download/v1/app.exe" -Destination $outFile # $ Alert +} + +function Curl_Native($outFile) { + # BAD - native curl with -o. + curl -sL "https://github.com/example/project/releases/download/v1/app.tar.gz" -o $outFile # $ Alert +} + +function Wget_Native() { + # BAD - native wget piped to tar (no output file at all). + wget "https://github.com/example/project/releases/download/v1/app.tar.gz" # $ MISSING: Alert +} + +function AzCopy($outFile) { + # BAD - azcopy / aria2c. + azcopy copy "https://github.com/Azure/azure-storage-azcopy/releases/download/v10.31.0/azcopy.zip" $outFile # $ MISSING: Alert +} + +function Wrapper_Function($outFile) { + # BAD - custom download wrapper; the real Invoke-WebRequest lives in a + # dot-sourced helper. + Download-File "https://github.com/example/project/releases/download/v1/app.exe" $outFile # $ MISSING: Alert +} + +# --------------------------------------------------------------------------- +# Verified by means other than Get-FileHash - should NOT be flagged. +# --------------------------------------------------------------------------- + +function Verified_With_CertUtil($outFile) { + # GOOD - certutil hash verification (now recognised as an integrity barrier). + $uri = "https://github.com/example/project/releases/download/v1/app.exe" + Invoke-WebRequest -Uri $uri -OutFile $outFile + $h = certutil -hashfile $outFile SHA256 + if ($h -notmatch "ABC123") { throw "hash mismatch" } +} + +# --------------------------------------------------------------------------- +# TRUE NEGATIVES: these must NOT be flagged. Either the download is verified, +# or it is not a trusted-host artifact download at all. No inline expectation +# means "no alert expected here". +# --------------------------------------------------------------------------- + +function Verified_With_ComputeHash($outFile) { + # GOOD - hash computed via [SHA256]::Create().ComputeHash over the bytes read + # from the downloaded file, then compared. + $uri = "https://github.com/example/project/releases/download/v1/app.exe" + Invoke-WebRequest -Uri $uri -OutFile $outFile + $sha = [System.Security.Cryptography.SHA256]::Create() + $bytes = [System.IO.File]::ReadAllBytes($outFile) + $hash = [System.BitConverter]::ToString($sha.ComputeHash($bytes)).Replace("-", "") + if ($hash -ne "ABC123") { throw "hash mismatch" } +} + +function Verified_With_Signature($outFile) { + # GOOD - Authenticode signature check on the downloaded file. + $uri = "https://github.com/example/project/releases/download/v1/app.exe" + Invoke-WebRequest -Uri $uri -OutFile $outFile + $sig = Get-AuthenticodeSignature -FilePath $outFile + if ($sig.Status -ne "Valid") { throw "bad signature" } +} + +function Verified_With_Cosign($outFile) { + # GOOD - external signature verification via cosign. + $uri = "https://github.com/example/project/releases/download/v1/app.tar.gz" + Invoke-WebRequest -Uri $uri -OutFile $outFile + cosign verify-blob --signature "$outFile.sig" $outFile +} + +function NonArtifact_Host($outFile) { + # GOOD - download from a host that is not a recognised artifact host. + Invoke-WebRequest -Uri "https://example.com/data/report.json" -OutFile $outFile +} + +function GitHub_Api_DataOnly() { + # GOOD - a GitHub API metadata call, not an artifact download. The response is + # only inspected, never written to disk or executed. + $r = Invoke-RestMethod -Uri "https://api.github.com/repos/example/project/releases/latest" + return $r.tag_name +} + +function Local_Copy($outFile) { + # GOOD - copying a local file; no network download, no trusted-host URL. + Copy-Item -Path "C:\artifacts\app.exe" -Destination $outFile +}