Skip to content

Commit f10abda

Browse files
chanel-ychanel-y
committed
added additional test cases
1 parent 48744da commit f10abda

3 files changed

Lines changed: 215 additions & 49 deletions

File tree

‎powershell/ql/src/queries/security/cwe-494/DownloadWithoutIntegrityCheck.ql‎

Lines changed: 102 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -15,62 +15,126 @@ import powershell
1515
import semmle.code.powershell.dataflow.DataFlow
1616
import semmle.code.powershell.dataflow.TaintTracking
1717

18-
predicate invokeGitHub(DataFlow::CallNode call, DataFlow::Node out) {
19-
exists(DataFlow::Node source, string s, DataFlow::Node uri |
20-
call.getAName() = ["Invoke-WebRequest", "iwr", "Invoke-RestMethod", "irm", "curl", "wget"] and
21-
uri = call.getNamedArgument("uri") and
22-
TaintTracking::localTaint(source, uri) and
23-
s.toLowerCase()
24-
.matches([
25-
"%github%",
26-
"%gitlab%",
27-
"%bitbucket%",
28-
"%sourceforge%",
29-
"%powershellgallery%",
30-
"%nuget%",
31-
"%npmjs%",
32-
"%pypi%",
33-
"%repo1.maven%",
34-
"%repo.maven.apache%",
35-
"%blob.core.windows%",
36-
"%amazonaws%",
37-
"%googleapis%",
38-
"%azure%",
39-
"%visualstudio%",
40-
"%jfrog%",
41-
"%artifactory%"
42-
]) and
18+
/** Holds if `s` looks like a URL pointing at a trusted artifact host. */
19+
bindingset[s]
20+
predicate isTrustedArtifactHost(string s) {
21+
s.toLowerCase()
22+
.matches([
23+
"%github%",
24+
"%gitlab%",
25+
"%bitbucket%",
26+
"%sourceforge%",
27+
"%powershellgallery%",
28+
"%nuget%",
29+
"%npmjs%",
30+
"%pypi%",
31+
"%repo1.maven%",
32+
"%repo.maven.apache%",
33+
"%blob.core.windows%",
34+
"%amazonaws%",
35+
"%googleapis%",
36+
"%azure%",
37+
"%visualstudio%",
38+
"%jfrog%",
39+
"%artifactory%"
40+
])
41+
}
42+
43+
/** Holds if `node` is tainted by a string constant that looks like an artifact URL. */
44+
predicate isArtifactUrl(DataFlow::Node node) {
45+
exists(DataFlow::Node source, string s |
46+
TaintTracking::localTaint(source, node) and
4347
s =
4448
[
4549
source.asExpr().getValue().asString(),
4650
source.asExpr().getExpr().(ExpandableStringExpr).getUnexpandedValue()
4751
] and
48-
out = call.getNamedArgument("outfile")
52+
isTrustedArtifactHost(s)
4953
)
5054
}
5155

56+
/**
57+
* Holds if `call` downloads an artifact, where `url` is the argument that
58+
* carries the (trusted-host) download URL.
59+
*
60+
* This covers cmdlets and their aliases (`Invoke-WebRequest`/`iwr`,
61+
* `Invoke-RestMethod`/`irm`, `Start-BitsTransfer`), native download tools
62+
* (`curl`, `wget`, `azcopy`, `aria2c`) and the .NET `WebClient`/`HttpClient`
63+
* download methods. The URL may be passed as a named argument (e.g. `-Uri`,
64+
* `-Source`), positionally, or as a method argument.
65+
*/
66+
predicate downloadCall(DataFlow::CallNode call, DataFlow::Node url) {
67+
call.getAName() =
68+
[
69+
// cmdlets and aliases
70+
"Invoke-WebRequest", "iwr", "Invoke-RestMethod", "irm", "Start-BitsTransfer",
71+
// native command-line download tools
72+
"curl", "curl.exe", "wget", "wget.exe", "azcopy", "azcopy.exe", "aria2c", "aria2c.exe",
73+
// .NET WebClient / HttpClient download methods
74+
"DownloadFile", "DownloadFileAsync", "DownloadFileTaskAsync", "DownloadData",
75+
"DownloadDataAsync", "DownloadDataTaskAsync", "DownloadString", "DownloadStringAsync",
76+
"GetByteArrayAsync", "GetStreamAsync"
77+
] and
78+
url = call.getAnArgument() and
79+
isArtifactUrl(url)
80+
}
81+
82+
/**
83+
* Gets the argument of `call` that names the file the artifact is written to,
84+
* if any. Downloads that consume the response inline (e.g. `irm ... | iex`)
85+
* have no such argument.
86+
*/
87+
DataFlow::Node getOutFileArg(DataFlow::CallNode call) {
88+
downloadCall(call, _) and
89+
(
90+
result =
91+
call.getNamedArgument([
92+
"outfile", "destination", "outputfile", "outpath", "literalpath", "path", "o"
93+
])
94+
or
95+
// WebClient.DownloadFile(url, destinationFile): the destination is the 2nd argument.
96+
call.getAName() = ["DownloadFile", "DownloadFileAsync", "DownloadFileTaskAsync"] and
97+
result = call.getArgument(1)
98+
)
99+
}
100+
101+
/**
102+
* Holds if `check` verifies the integrity of the file referred to by `file`,
103+
* by computing/comparing a hash or by checking a signature.
104+
*/
105+
predicate integrityCheck(DataFlow::CallNode check, DataFlow::Node file) {
106+
check.getAName() =
107+
[
108+
"Get-FileHash", "gfh", // hash a file
109+
"certutil", "certutil.exe", // certutil -hashfile <file> SHA256
110+
"ComputeHash", // [SHA256]::Create().ComputeHash(...)
111+
"Get-AuthenticodeSignature", "Test-FileCatalog", // signature / catalog checks
112+
"cosign", "cosign.exe", "gpg", "gpg.exe" // external signature verification
113+
] and
114+
file = check.getAnArgument()
115+
}
116+
52117
module Conf implements DataFlow::ConfigSig {
53-
predicate isSource(DataFlow::Node source) { invokeGitHub(_, source) }
118+
predicate isSource(DataFlow::Node source) { source = getOutFileArg(_) }
54119

55-
predicate isSink(DataFlow::Node sink) {
56-
exists(DataFlow::CallNode hasher |
57-
hasher.getAName() = ["Get-FileHash", "gfh"] and
58-
sink = hasher.getNamedArgument(["path", "literalpath"])
59-
)
60-
}
120+
predicate isSink(DataFlow::Node sink) { integrityCheck(_, sink) }
61121
}
62122

63123
module Flow = DataFlow::Global<Conf>;
64124

65-
predicate isHashed(DataFlow::Node out) {
125+
/** Holds if the downloaded file `out` flows to an integrity check. */
126+
predicate isVerified(DataFlow::Node out) {
66127
exists(Flow::PathNode source |
67128
source.getNode() = out and
68129
source.isSource()
69130
)
70131
}
71132

72-
from DataFlow::CallNode call, DataFlow::Node out
133+
from DataFlow::CallNode call
73134
where
74-
invokeGitHub(call, out) and
75-
not isHashed(out)
76-
select call, "This downloads an artifact from GitHub without checking its hash."
135+
downloadCall(call, _) and
136+
// Report unless the file that was downloaded is later verified. A download
137+
// with no output-file argument cannot be hash-verified, so it is reported.
138+
not exists(DataFlow::Node out | out = getOutFileArg(call) and isVerified(out))
139+
select call,
140+
"This downloads an artifact without verifying its integrity (e.g. a hash or signature check)."

‎powershell/ql/test/query-tests/security/cwe-494/DownloadWithoutIntegrityCheck.expected‎

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1 +1,11 @@
1-
| test.ps1:3:3:3:47 | Call to invoke-webrequest | This downloads an artifact from GitHub without checking its hash. |
1+
| 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). |
2+
| 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). |
3+
| test.ps1:45:3:45:89 | Call to iwr | This downloads an artifact without verifying its integrity (e.g. a hash or signature check). |
4+
| test.ps1:50:8:50:101 | Call to invoke-webrequest | This downloads an artifact without verifying its integrity (e.g. a hash or signature check). |
5+
| test.ps1:56:3:56:75 | Call to irm | This downloads an artifact without verifying its integrity (e.g. a hash or signature check). |
6+
| test.ps1:66:3:66:95 | Call to downloadfile | This downloads an artifact without verifying its integrity (e.g. a hash or signature check). |
7+
| test.ps1:71:3:71:118 | Call to downloadfile | This downloads an artifact without verifying its integrity (e.g. a hash or signature check). |
8+
| 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). |
9+
| test.ps1:81:3:81:91 | Call to curl | This downloads an artifact without verifying its integrity (e.g. a hash or signature check). |
10+
| test.ps1:86:3:86:75 | Call to wget | This downloads an artifact without verifying its integrity (e.g. a hash or signature check). |
11+
| test.ps1:91:3:91:108 | Call to azcopy | This downloads an artifact without verifying its integrity (e.g. a hash or signature check). |

‎powershell/ql/test/query-tests/security/cwe-494/test.ps1‎

Lines changed: 102 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,18 +1,110 @@
1-
function Test1($outFile) {
1+
# CWE-494 - Download of a GitHub release artifact without an integrity check.
2+
#
3+
# Each function below is one download idiom. It documents, as inline
4+
# expectations, which idioms the query catches and which it still misses:
5+
#
6+
# # $ Alert -> the query reports here (expected true positive)
7+
# # $ MISSING: Alert -> the query SHOULD report here but does not yet
8+
# (known false negative)
9+
# # $ SPURIOUS: Alert -> the query reports here but should not
10+
# (known false positive)
11+
#
12+
# When the query is improved to close a gap, flip MISSING -> Alert (or remove
13+
# SPURIOUS) and re-accept the .expected file.
14+
15+
# ---------------------------------------------------------------------------
16+
# COVERED today: a recognised cmdlet with a NAMED -Uri and a NAMED -OutFile.
17+
# ---------------------------------------------------------------------------
18+
19+
function Iwr_NamedUri_OutFile($outFile) {
20+
# BAD - baseline; download from github with no hash check.
221
$uri = "https://github.com/example/project/releases/download/v1.2.3/installer.exe"
322
Invoke-WebRequest -Uri $uri -OutFile $outFile # $ Alert
423
}
524

6-
function Test2($outFile) {
25+
function Iwr_NamedUri_OutFile_Hashed($outFile) {
26+
# GOOD - Get-FileHash barrier on the downloaded file (correctly NOT flagged).
727
$uri = "https://github.com/example/project/releases/download/v1.2.3/installer.exe"
8-
$expectedHash = "8c954cd9b6c8f8180a05f1de66ee555f0c44b4c6738120785d827521bb8d54df"
28+
$expected = "8c954cd9b6c8f8180a05f1de66ee555f0c44b4c6738120785d827521bb8d54df"
29+
Invoke-WebRequest -Uri $uri -OutFile $outFile
30+
$actual = (Get-FileHash -Path $outFile -Algorithm SHA256).Hash
31+
if ($actual -ne $expected) { Remove-Item $outFile -Force; throw "hash mismatch" }
32+
}
33+
34+
# ---------------------------------------------------------------------------
35+
# MISSING: argument-shape variants of Invoke-WebRequest / Invoke-RestMethod.
36+
# ---------------------------------------------------------------------------
37+
38+
function Iwr_PositionalUri($outFile) {
39+
# BAD - URL passed positionally, not as -Uri.
40+
Invoke-WebRequest "https://github.com/example/project/releases/download/v1/app.exe" -OutFile $outFile # $ Alert
41+
}
42+
43+
function Iwr_Alias_Positional($outFile) {
44+
# BAD - iwr alias + positional URL.
45+
iwr "https://github.com/example/project/releases/download/v1/app.zip" -OutFile $outFile # $ Alert
46+
}
47+
48+
function Iwr_NoOutFile_PipedToIex() {
49+
# BAD - no -OutFile; content executed inline.
50+
$r = Invoke-WebRequest -Uri "https://github.com/example/project/releases/download/v1/bootstrap.ps1" # $ Alert
51+
Invoke-Expression $r.Content
52+
}
953

10-
Invoke-WebRequest -Uri $uri -OutFile $outFile # GOOD
54+
function Irm_NoOutFile() {
55+
# BAD - Invoke-RestMethod returns content, no -OutFile.
56+
irm "https://github.com/example/project/releases/download/v1/install.ps1" | Invoke-Expression # $ Alert
57+
}
1158

12-
$actualHash = (Get-FileHash -Path $outFile -Algorithm SHA256).Hash
59+
# ---------------------------------------------------------------------------
60+
# MISSING: download sinks that are not in the cmdlet name list at all.
61+
# ---------------------------------------------------------------------------
1362

14-
if ($actualHash -ne $expectedHash) {
15-
Remove-Item -Path $outFile -Force
16-
throw "Integrity check failed: '$outFile' does not match the expected hash."
17-
}
18-
}
63+
function WebClient_DownloadFile($outFile) {
64+
# BAD - System.Net.WebClient.DownloadFile.
65+
$wc = New-Object System.Net.WebClient
66+
$wc.DownloadFile("https://github.com/example/project/releases/download/v1/app.exe", $outFile) # $ Alert
67+
}
68+
69+
function WebClient_Inline($outFile) {
70+
# BAD - inline (New-Object Net.WebClient).DownloadFile(...).
71+
(New-Object Net.WebClient).DownloadFile("https://github.com/example/project/releases/download/v1/app.exe", $outFile) # $ Alert
72+
}
73+
74+
function Bits_Transfer($outFile) {
75+
# BAD - Start-BitsTransfer.
76+
Start-BitsTransfer -Source "https://github.com/example/project/releases/download/v1/app.exe" -Destination $outFile # $ Alert
77+
}
78+
79+
function Curl_Native($outFile) {
80+
# BAD - native curl with -o.
81+
curl -sL "https://github.com/example/project/releases/download/v1/app.tar.gz" -o $outFile # $ Alert
82+
}
83+
84+
function Wget_Native() {
85+
# BAD - native wget piped to tar (no output file at all).
86+
wget "https://github.com/example/project/releases/download/v1/app.tar.gz" # $ Alert
87+
}
88+
89+
function AzCopy($outFile) {
90+
# BAD - azcopy / aria2c.
91+
azcopy copy "https://github.com/Azure/azure-storage-azcopy/releases/download/v10.31.0/azcopy.zip" $outFile # $ Alert
92+
}
93+
94+
function Wrapper_Function($outFile) {
95+
# BAD - custom download wrapper; the real Invoke-WebRequest lives in a
96+
# dot-sourced helper.
97+
Download-File "https://github.com/example/project/releases/download/v1/app.exe" $outFile # $ MISSING: Alert
98+
}
99+
100+
# ---------------------------------------------------------------------------
101+
# Verified by means other than Get-FileHash - should NOT be flagged.
102+
# ---------------------------------------------------------------------------
103+
104+
function Verified_With_CertUtil($outFile) {
105+
# GOOD - certutil hash verification (now recognised as an integrity barrier).
106+
$uri = "https://github.com/example/project/releases/download/v1/app.exe"
107+
Invoke-WebRequest -Uri $uri -OutFile $outFile
108+
$h = certutil -hashfile $outFile SHA256
109+
if ($h -notmatch "ABC123") { throw "hash mismatch" }
110+
}

0 commit comments

Comments
 (0)