Skip to content

Commit 0c40e14

Browse files
martincostellomartincostello
authored
🐛 Trust pinned GitHub download URLs (#3694)
* Trust pinned GitHub download URLs Trust files that are downloaded from `raw.githubusercontent.com` where the file's ref is a Git SHA and therefore immutable. Resolves #3339. Signed-off-by: martincostello <[email protected]> * Move logic to function - Add `hasUnpinnedURLs` function. - Add test cases for different URLs. Signed-off-by: martincostello <[email protected]> * Fix formatting Appease the linter. Signed-off-by: martincostello <[email protected]> * Suppress lint warnings Suppress warning on three long URLs. Signed-off-by: martincostello <[email protected]> * Address peer review Address peer review feedback. Signed-off-by: martincostello <[email protected]> * Fix lint warning Fix lint warning. Signed-off-by: martincostello <[email protected]>
1 parent ce0b54e commit 0c40e14

8 files changed

+240
-5
lines changed

‎checks/raw/pinned_dependencies_test.go

Lines changed: 39 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -728,6 +728,24 @@ func TestDockerfileInsecureDownloadsLineNumber(t *testing.T) {
728728
endLine: 64,
729729
t: checker.DependencyUseTypePipCommand,
730730
},
731+
{
732+
snippet: `bash <(curl --silent --show-error "https://raw.githubusercontent.com/rhysd/actionlint/main/scripts/download-actionlint.bash")`,
733+
startLine: 68,
734+
endLine: 68,
735+
t: checker.DependencyUseTypeDownloadThenRun,
736+
},
737+
{
738+
snippet: "curl -sSL https://dot.net/v1/dotnet-install.sh | bash /dev/stdin",
739+
startLine: 69,
740+
endLine: 69,
741+
t: checker.DependencyUseTypeDownloadThenRun,
742+
},
743+
{
744+
snippet: "curl -sSL https://raw.githubusercontent.com/dotnet/install-scripts/main/src/dotnet-install.sh | bash /dev/stdin",
745+
startLine: 70,
746+
endLine: 70,
747+
t: checker.DependencyUseTypeDownloadThenRun,
748+
},
731749
},
732750
},
733751
{
@@ -975,6 +993,24 @@ func TestShellscriptInsecureDownloadsLineNumber(t *testing.T) {
975993
endLine: 64,
976994
t: checker.DependencyUseTypeNugetCommand,
977995
},
996+
{
997+
snippet: `bash <(curl --silent --show-error "https://raw.githubusercontent.com/rhysd/actionlint/main/scripts/download-actionlint.bash")`,
998+
startLine: 69,
999+
endLine: 69,
1000+
t: checker.DependencyUseTypeDownloadThenRun,
1001+
},
1002+
{
1003+
snippet: "curl -sSL https://dot.net/v1/dotnet-install.sh | bash /dev/stdin",
1004+
startLine: 70,
1005+
endLine: 70,
1006+
t: checker.DependencyUseTypeDownloadThenRun,
1007+
},
1008+
{
1009+
snippet: "curl -sSL https://raw.githubusercontent.com/dotnet/install-scripts/main/src/dotnet-install.sh | bash /dev/stdin",
1010+
startLine: 71,
1011+
endLine: 71,
1012+
t: checker.DependencyUseTypeDownloadThenRun,
1013+
},
9781014
},
9791015
},
9801016
}
@@ -1079,7 +1115,7 @@ func TestDockerfileScriptDownload(t *testing.T) {
10791115
{
10801116
name: "curl | sh",
10811117
filename: "./testdata/Dockerfile-curl-sh",
1082-
unpinned: 4,
1118+
unpinned: 5,
10831119
},
10841120
{
10851121
name: "empty file",
@@ -1096,7 +1132,7 @@ func TestDockerfileScriptDownload(t *testing.T) {
10961132
{
10971133
name: "wget | /bin/sh",
10981134
filename: "./testdata/Dockerfile-wget-bin-sh",
1099-
unpinned: 3,
1135+
unpinned: 4,
11001136
},
11011137
{
11021138
name: "wget no exec",
@@ -1262,7 +1298,7 @@ func TestShellScriptDownload(t *testing.T) {
12621298
{
12631299
name: "bash script",
12641300
filename: "./testdata/script-bash",
1265-
unpinned: 7,
1301+
unpinned: 11,
12661302
},
12671303
{
12681304
name: "sh script 2",

‎checks/raw/shell_download_validate.go

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,8 @@ var downloadUtils = []string{
5757
"curl", "wget", "gsutil",
5858
}
5959

60+
var gitCommitHashRegex = regexp.MustCompile(`^[a-fA-F0-9]{40}$`)
61+
6062
func isBinaryName(expected, name string) bool {
6163
return strings.EqualFold(path.Base(name), expected)
6264
}
@@ -296,6 +298,36 @@ func getLine(startLine, endLine uint, node syntax.Node) (uint, uint) {
296298
startLine + node.Pos().Line()
297299
}
298300

301+
func hasUnpinnedURLs(cmd []string) bool {
302+
var urls []*url.URL
303+
304+
// Extract any URLs passed to the download utility
305+
for _, s := range cmd {
306+
u, err := url.ParseRequestURI(s)
307+
if err == nil {
308+
urls = append(urls, u)
309+
}
310+
}
311+
312+
// Look for any URLs which are pinned to a GitHub SHA
313+
var pinned []*url.URL
314+
for _, u := range urls {
315+
// Look for a URL of the form: https://raw.githubusercontent.com/{owner}/{repo}/{ref}/{path}
316+
if u.Scheme == "https" && u.Host == "raw.githubusercontent.com" {
317+
segments := strings.Split(u.Path, "/")
318+
if len(segments) > 4 && gitCommitHashRegex.MatchString(segments[3]) {
319+
pinned = append(pinned, u)
320+
}
321+
}
322+
}
323+
324+
if len(pinned) > 0 && len(urls) == len(pinned) {
325+
return false
326+
}
327+
328+
return true
329+
}
330+
299331
func collectFetchPipeExecute(startLine, endLine uint, node syntax.Node, cmd, pathfn string,
300332
r *checker.PinningDependenciesData,
301333
) {
@@ -327,6 +359,10 @@ func collectFetchPipeExecute(startLine, endLine uint, node syntax.Node, cmd, pat
327359
return
328360
}
329361

362+
if !hasUnpinnedURLs(leftStmt) {
363+
return
364+
}
365+
330366
startLine, endLine = getLine(startLine, endLine, node)
331367

332368
r.Dependencies = append(r.Dependencies,

‎checks/raw/shell_download_validate_test.go

Lines changed: 128 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -312,3 +312,131 @@ func Test_isNpmUnpinnedDownload(t *testing.T) {
312312
})
313313
}
314314
}
315+
316+
func Test_hasUnpinnedURLs(t *testing.T) {
317+
t.Parallel()
318+
type args struct {
319+
cmd []string
320+
}
321+
tests := []struct {
322+
name string
323+
args args
324+
expected bool
325+
}{
326+
{
327+
name: "Unpinned URL",
328+
args: args{
329+
cmd: []string{
330+
"curl",
331+
"-sSL",
332+
"https://dot.net/v1/dotnet-install.sh",
333+
},
334+
},
335+
expected: true,
336+
},
337+
{
338+
name: "GitHub content URL but no path",
339+
args: args{
340+
cmd: []string{
341+
"wget",
342+
"-0",
343+
"-",
344+
"https://raw.githubusercontent.com",
345+
},
346+
},
347+
expected: true,
348+
},
349+
{
350+
name: "GitHub content URL but no ref",
351+
args: args{
352+
cmd: []string{
353+
"wget",
354+
"-0",
355+
"-",
356+
"https://raw.githubusercontent.com/dotnet/install-scripts",
357+
},
358+
},
359+
expected: true,
360+
},
361+
{
362+
name: "Unpinned GitHub content URL",
363+
args: args{
364+
cmd: []string{
365+
"curl",
366+
"-sSL",
367+
"https://raw.githubusercontent.com/dotnet/install-scripts/main/src/dotnet-install.sh",
368+
},
369+
},
370+
expected: true,
371+
},
372+
{
373+
name: "Pinned GitHub content URL but invalid SHA",
374+
args: args{
375+
cmd: []string{
376+
"wget",
377+
"-0",
378+
"-",
379+
"https://raw.githubusercontent.com/dotnet/install-scripts/zzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzz/src/dotnet-install.sh", //nolint:lll
380+
},
381+
},
382+
expected: true,
383+
},
384+
{
385+
name: "Pinned GitHub content URL but no file path",
386+
args: args{
387+
cmd: []string{
388+
"wget",
389+
"-0",
390+
"-",
391+
"https://raw.githubusercontent.com/dotnet/install-scripts/5b142a1e445a6f060d6430b661408989e9580b85",
392+
},
393+
},
394+
expected: true,
395+
},
396+
{
397+
name: "Pinned GitHub content URL",
398+
args: args{
399+
cmd: []string{
400+
"wget",
401+
"-0",
402+
"-",
403+
"https://raw.githubusercontent.com/dotnet/install-scripts/5b142a1e445a6f060d6430b661408989e9580b85/src/dotnet-install.sh", //nolint:lll
404+
},
405+
},
406+
expected: false,
407+
},
408+
{
409+
name: "Pinned GitHub content URL but HTTP",
410+
args: args{
411+
cmd: []string{
412+
"wget",
413+
"-0",
414+
"-",
415+
"http://raw.githubusercontent.com/dotnet/install-scripts/5b142a1e445a6f060d6430b661408989e9580b85/src/dotnet-install.sh", //nolint:lll
416+
},
417+
},
418+
expected: true,
419+
},
420+
{
421+
name: "Pinned GitHub URL but not raw content",
422+
args: args{
423+
cmd: []string{
424+
"wget",
425+
"-0",
426+
"-",
427+
"https://github.com/dotnet/install-scripts/blob/5b142a1e445a6f060d6430b661408989e9580b85/src/dotnet-install.sh",
428+
},
429+
},
430+
expected: true,
431+
},
432+
}
433+
for _, tt := range tests {
434+
tt := tt // Re-initializing variable so it is not changed while executing the closure below
435+
t.Run(tt.name, func(t *testing.T) {
436+
t.Parallel()
437+
if actual := hasUnpinnedURLs(tt.args.cmd); actual != tt.expected {
438+
t.Errorf("hasUnpinnedURLs() = %v, expected %v for %v", actual, tt.expected, tt.name)
439+
}
440+
})
441+
}
442+
}

‎checks/raw/testdata/Dockerfile-curl-sh

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,5 +20,11 @@ RUN echo hello && curl -s file-with-sudo2 | sudo bash
2020
RUN echo hello && sudo curl -s file-with-sudo | bash | bla
2121
RUN ["echo", "hello", "&&", "curl", "-s", "/etc/file2", "|", "sh"]
2222

23+
# Unpinned
24+
RUN curl -sSL https://raw.githubusercontent.com/dotnet/install-scripts/main/src/dotnet-install.sh | bash /dev/stdin
25+
26+
# Pinned
27+
RUN curl -sSL https://raw.githubusercontent.com/dotnet/install-scripts/5b142a1e445a6f060d6430b661408989e9580b85/src/dotnet-install.sh | bash /dev/stdin
28+
2329
FROM scratch
2430
FROM python@sha256:45b23dee08af5e43a7fea6c4cf9c25ccf269ee113168c19722f87876677c5cb2

‎checks/raw/testdata/Dockerfile-download-lines

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -62,4 +62,12 @@ RUN pip install --no-deps -e . git+https://github.com/username/repo.git
6262
RUN pip install --no-deps -e . git+https://github.com/username/repo.git@0123456789abcdef0123456789abcdef01234567#egg=package
6363

6464
RUN python -m pip install --no-deps -e git+https://github.com/username/repo.git
65-
RUN python -m pip install --no-deps -e git+https://github.com/username/repo.git@0123456789abcdef0123456789abcdef01234567#egg=package
65+
RUN python -m pip install --no-deps -e git+https://github.com/username/repo.git@0123456789abcdef0123456789abcdef01234567#egg=package
66+
67+
# Unpinned
68+
RUN bash <(curl --silent --show-error "https://raw.githubusercontent.com/rhysd/actionlint/main/scripts/download-actionlint.bash")
69+
RUN curl -sSL https://dot.net/v1/dotnet-install.sh | bash /dev/stdin
70+
RUN curl -sSL https://raw.githubusercontent.com/dotnet/install-scripts/main/src/dotnet-install.sh | bash /dev/stdin
71+
# Pinned
72+
RUN bash <(curl --silent --show-error "https://raw.githubusercontent.com/rhysd/actionlint/7b75d16d41920ec126e6f3269db0c6f3ab613c38/scripts/download-actionlint.bash")
73+
RUN curl -sSL https://raw.githubusercontent.com/dotnet/install-scripts/5b142a1e445a6f060d6430b661408989e9580b85/src/dotnet-install.sh | bash /dev/stdin

‎checks/raw/testdata/Dockerfile-wget-bin-sh

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,5 +19,11 @@ RUN echo hello && wget -0 - http://ifconfig.co/json | /bin/sh
1919
RUN ["echo", "hello", "&&", "wget", "-0", "-", "http://ifconfig.co/json", "|", "/bin/sh"]
2020
RUN ["sh", "-c", "\"wget -0 - http://ifconfig.co/json | /bin/sh\""]
2121

22+
# Unpinned
23+
RUN wget -0 - https://raw.githubusercontent.com/dotnet/install-scripts/main/src/dotnet-install.sh | /bin/sh
24+
25+
# Pinned
26+
RUN wget -0 - https://raw.githubusercontent.com/dotnet/install-scripts/5b142a1e445a6f060d6430b661408989e9580b85/src/dotnet-install.sh | /bin/sh
27+
2228
FROM scratch
2329
FROM python@sha256:45b23dee08af5e43a7fea6c4cf9c25ccf269ee113168c19722f87876677c5cb2

‎checks/raw/testdata/script-bash

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -49,4 +49,13 @@ bash <(wget -qO- http://website.com/my-script.sh)
4949
wget http://file-with-sudo -O /tmp/file3
5050
bash /tmp/file3
5151

52+
bash <(curl --silent --show-error "https://raw.githubusercontent.com/rhysd/actionlint/main/scripts/download-actionlint.bash")
53+
curl -sSL https://dot.net/v1/dotnet-install.sh | bash /dev/stdin
54+
curl -sSL https://raw.githubusercontent.com/dotnet/install-scripts/main/src/dotnet-install.sh | bash /dev/stdin
55+
RUN bash <(curl --silent --show-error "https://raw.githubusercontent.com/rhysd/actionlint/7b75d16d41920ec126e6f3269db0c6f3ab613c38/scripts/download-actionlint.bash")
56+
RUN curl -sSL https://raw.githubusercontent.com/dotnet/install-scripts/5b142a1e445a6f060d6430b661408989e9580b85/src/dotnet-install.sh | bash /dev/stdin
57+
58+
wget https://dot.net/v1/dotnet-install.sh -O /tmp/file4
59+
bash /tmp/file4
60+
5261
date

‎checks/raw/testdata/shell-download-lines.sh

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -64,4 +64,10 @@ dotnet add package some-package
6464
dotnet add SomeProject package some-package
6565
dotnet build
6666
dotnet add package some-package -v 1.2.3
67-
dotnet add package some-package --version 1.2.3
67+
dotnet add package some-package --version 1.2.3
68+
69+
bash <(curl --silent --show-error "https://raw.githubusercontent.com/rhysd/actionlint/main/scripts/download-actionlint.bash")
70+
curl -sSL https://dot.net/v1/dotnet-install.sh | bash /dev/stdin
71+
curl -sSL https://raw.githubusercontent.com/dotnet/install-scripts/main/src/dotnet-install.sh | bash /dev/stdin
72+
RUN bash <(curl --silent --show-error "https://raw.githubusercontent.com/rhysd/actionlint/7b75d16d41920ec126e6f3269db0c6f3ab613c38/scripts/download-actionlint.bash")
73+
RUN curl -sSL https://raw.githubusercontent.com/dotnet/install-scripts/5b142a1e445a6f060d6430b661408989e9580b85/src/dotnet-install.sh | bash /dev/stdin

0 commit comments

Comments
 (0)