From 0c40e14b75a865ff030e9b4884bdc97b1eac3e64 Mon Sep 17 00:00:00 2001 From: Martin Costello Date: Thu, 30 Nov 2023 19:26:47 +0000 Subject: [PATCH] :bug: 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 * Move logic to function - Add `hasUnpinnedURLs` function. - Add test cases for different URLs. Signed-off-by: martincostello * Fix formatting Appease the linter. Signed-off-by: martincostello * Suppress lint warnings Suppress warning on three long URLs. Signed-off-by: martincostello * Address peer review Address peer review feedback. Signed-off-by: martincostello * Fix lint warning Fix lint warning. Signed-off-by: martincostello --- checks/raw/pinned_dependencies_test.go | 42 +++++- checks/raw/shell_download_validate.go | 36 +++++ checks/raw/shell_download_validate_test.go | 128 ++++++++++++++++++ checks/raw/testdata/Dockerfile-curl-sh | 6 + checks/raw/testdata/Dockerfile-download-lines | 10 +- checks/raw/testdata/Dockerfile-wget-bin-sh | 6 + checks/raw/testdata/script-bash | 9 ++ checks/raw/testdata/shell-download-lines.sh | 8 +- 8 files changed, 240 insertions(+), 5 deletions(-) diff --git a/checks/raw/pinned_dependencies_test.go b/checks/raw/pinned_dependencies_test.go index bc755c8dba1..3800006d9c0 100644 --- a/checks/raw/pinned_dependencies_test.go +++ b/checks/raw/pinned_dependencies_test.go @@ -728,6 +728,24 @@ func TestDockerfileInsecureDownloadsLineNumber(t *testing.T) { endLine: 64, t: checker.DependencyUseTypePipCommand, }, + { + snippet: `bash <(curl --silent --show-error "https://raw.githubusercontent.com/rhysd/actionlint/main/scripts/download-actionlint.bash")`, + startLine: 68, + endLine: 68, + t: checker.DependencyUseTypeDownloadThenRun, + }, + { + snippet: "curl -sSL https://dot.net/v1/dotnet-install.sh | bash /dev/stdin", + startLine: 69, + endLine: 69, + t: checker.DependencyUseTypeDownloadThenRun, + }, + { + snippet: "curl -sSL https://raw.githubusercontent.com/dotnet/install-scripts/main/src/dotnet-install.sh | bash /dev/stdin", + startLine: 70, + endLine: 70, + t: checker.DependencyUseTypeDownloadThenRun, + }, }, }, { @@ -975,6 +993,24 @@ func TestShellscriptInsecureDownloadsLineNumber(t *testing.T) { endLine: 64, t: checker.DependencyUseTypeNugetCommand, }, + { + snippet: `bash <(curl --silent --show-error "https://raw.githubusercontent.com/rhysd/actionlint/main/scripts/download-actionlint.bash")`, + startLine: 69, + endLine: 69, + t: checker.DependencyUseTypeDownloadThenRun, + }, + { + snippet: "curl -sSL https://dot.net/v1/dotnet-install.sh | bash /dev/stdin", + startLine: 70, + endLine: 70, + t: checker.DependencyUseTypeDownloadThenRun, + }, + { + snippet: "curl -sSL https://raw.githubusercontent.com/dotnet/install-scripts/main/src/dotnet-install.sh | bash /dev/stdin", + startLine: 71, + endLine: 71, + t: checker.DependencyUseTypeDownloadThenRun, + }, }, }, } @@ -1079,7 +1115,7 @@ func TestDockerfileScriptDownload(t *testing.T) { { name: "curl | sh", filename: "./testdata/Dockerfile-curl-sh", - unpinned: 4, + unpinned: 5, }, { name: "empty file", @@ -1096,7 +1132,7 @@ func TestDockerfileScriptDownload(t *testing.T) { { name: "wget | /bin/sh", filename: "./testdata/Dockerfile-wget-bin-sh", - unpinned: 3, + unpinned: 4, }, { name: "wget no exec", @@ -1262,7 +1298,7 @@ func TestShellScriptDownload(t *testing.T) { { name: "bash script", filename: "./testdata/script-bash", - unpinned: 7, + unpinned: 11, }, { name: "sh script 2", diff --git a/checks/raw/shell_download_validate.go b/checks/raw/shell_download_validate.go index d5fd530142e..b2149bbf656 100644 --- a/checks/raw/shell_download_validate.go +++ b/checks/raw/shell_download_validate.go @@ -57,6 +57,8 @@ var downloadUtils = []string{ "curl", "wget", "gsutil", } +var gitCommitHashRegex = regexp.MustCompile(`^[a-fA-F0-9]{40}$`) + func isBinaryName(expected, name string) bool { return strings.EqualFold(path.Base(name), expected) } @@ -296,6 +298,36 @@ func getLine(startLine, endLine uint, node syntax.Node) (uint, uint) { startLine + node.Pos().Line() } +func hasUnpinnedURLs(cmd []string) bool { + var urls []*url.URL + + // Extract any URLs passed to the download utility + for _, s := range cmd { + u, err := url.ParseRequestURI(s) + if err == nil { + urls = append(urls, u) + } + } + + // Look for any URLs which are pinned to a GitHub SHA + var pinned []*url.URL + for _, u := range urls { + // Look for a URL of the form: https://raw.githubusercontent.com/{owner}/{repo}/{ref}/{path} + if u.Scheme == "https" && u.Host == "raw.githubusercontent.com" { + segments := strings.Split(u.Path, "/") + if len(segments) > 4 && gitCommitHashRegex.MatchString(segments[3]) { + pinned = append(pinned, u) + } + } + } + + if len(pinned) > 0 && len(urls) == len(pinned) { + return false + } + + return true +} + func collectFetchPipeExecute(startLine, endLine uint, node syntax.Node, cmd, pathfn string, r *checker.PinningDependenciesData, ) { @@ -327,6 +359,10 @@ func collectFetchPipeExecute(startLine, endLine uint, node syntax.Node, cmd, pat return } + if !hasUnpinnedURLs(leftStmt) { + return + } + startLine, endLine = getLine(startLine, endLine, node) r.Dependencies = append(r.Dependencies, diff --git a/checks/raw/shell_download_validate_test.go b/checks/raw/shell_download_validate_test.go index d1982216a26..e7f615cac7a 100644 --- a/checks/raw/shell_download_validate_test.go +++ b/checks/raw/shell_download_validate_test.go @@ -312,3 +312,131 @@ func Test_isNpmUnpinnedDownload(t *testing.T) { }) } } + +func Test_hasUnpinnedURLs(t *testing.T) { + t.Parallel() + type args struct { + cmd []string + } + tests := []struct { + name string + args args + expected bool + }{ + { + name: "Unpinned URL", + args: args{ + cmd: []string{ + "curl", + "-sSL", + "https://dot.net/v1/dotnet-install.sh", + }, + }, + expected: true, + }, + { + name: "GitHub content URL but no path", + args: args{ + cmd: []string{ + "wget", + "-0", + "-", + "https://raw.githubusercontent.com", + }, + }, + expected: true, + }, + { + name: "GitHub content URL but no ref", + args: args{ + cmd: []string{ + "wget", + "-0", + "-", + "https://raw.githubusercontent.com/dotnet/install-scripts", + }, + }, + expected: true, + }, + { + name: "Unpinned GitHub content URL", + args: args{ + cmd: []string{ + "curl", + "-sSL", + "https://raw.githubusercontent.com/dotnet/install-scripts/main/src/dotnet-install.sh", + }, + }, + expected: true, + }, + { + name: "Pinned GitHub content URL but invalid SHA", + args: args{ + cmd: []string{ + "wget", + "-0", + "-", + "https://raw.githubusercontent.com/dotnet/install-scripts/zzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzz/src/dotnet-install.sh", //nolint:lll + }, + }, + expected: true, + }, + { + name: "Pinned GitHub content URL but no file path", + args: args{ + cmd: []string{ + "wget", + "-0", + "-", + "https://raw.githubusercontent.com/dotnet/install-scripts/5b142a1e445a6f060d6430b661408989e9580b85", + }, + }, + expected: true, + }, + { + name: "Pinned GitHub content URL", + args: args{ + cmd: []string{ + "wget", + "-0", + "-", + "https://raw.githubusercontent.com/dotnet/install-scripts/5b142a1e445a6f060d6430b661408989e9580b85/src/dotnet-install.sh", //nolint:lll + }, + }, + expected: false, + }, + { + name: "Pinned GitHub content URL but HTTP", + args: args{ + cmd: []string{ + "wget", + "-0", + "-", + "http://raw.githubusercontent.com/dotnet/install-scripts/5b142a1e445a6f060d6430b661408989e9580b85/src/dotnet-install.sh", //nolint:lll + }, + }, + expected: true, + }, + { + name: "Pinned GitHub URL but not raw content", + args: args{ + cmd: []string{ + "wget", + "-0", + "-", + "https://github.com/dotnet/install-scripts/blob/5b142a1e445a6f060d6430b661408989e9580b85/src/dotnet-install.sh", + }, + }, + expected: true, + }, + } + for _, tt := range tests { + tt := tt // Re-initializing variable so it is not changed while executing the closure below + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + if actual := hasUnpinnedURLs(tt.args.cmd); actual != tt.expected { + t.Errorf("hasUnpinnedURLs() = %v, expected %v for %v", actual, tt.expected, tt.name) + } + }) + } +} diff --git a/checks/raw/testdata/Dockerfile-curl-sh b/checks/raw/testdata/Dockerfile-curl-sh index e80a7e194dc..19a9ab0dfac 100644 --- a/checks/raw/testdata/Dockerfile-curl-sh +++ b/checks/raw/testdata/Dockerfile-curl-sh @@ -20,5 +20,11 @@ RUN echo hello && curl -s file-with-sudo2 | sudo bash RUN echo hello && sudo curl -s file-with-sudo | bash | bla RUN ["echo", "hello", "&&", "curl", "-s", "/etc/file2", "|", "sh"] +# Unpinned +RUN curl -sSL https://raw.githubusercontent.com/dotnet/install-scripts/main/src/dotnet-install.sh | bash /dev/stdin + +# Pinned +RUN curl -sSL https://raw.githubusercontent.com/dotnet/install-scripts/5b142a1e445a6f060d6430b661408989e9580b85/src/dotnet-install.sh | bash /dev/stdin + FROM scratch FROM python@sha256:45b23dee08af5e43a7fea6c4cf9c25ccf269ee113168c19722f87876677c5cb2 \ No newline at end of file diff --git a/checks/raw/testdata/Dockerfile-download-lines b/checks/raw/testdata/Dockerfile-download-lines index 17bba340329..386dc20f427 100644 --- a/checks/raw/testdata/Dockerfile-download-lines +++ b/checks/raw/testdata/Dockerfile-download-lines @@ -62,4 +62,12 @@ RUN pip install --no-deps -e . git+https://github.com/username/repo.git RUN pip install --no-deps -e . git+https://github.com/username/repo.git@0123456789abcdef0123456789abcdef01234567#egg=package RUN python -m pip install --no-deps -e git+https://github.com/username/repo.git -RUN python -m pip install --no-deps -e git+https://github.com/username/repo.git@0123456789abcdef0123456789abcdef01234567#egg=package \ No newline at end of file +RUN python -m pip install --no-deps -e git+https://github.com/username/repo.git@0123456789abcdef0123456789abcdef01234567#egg=package + +# Unpinned +RUN bash <(curl --silent --show-error "https://raw.githubusercontent.com/rhysd/actionlint/main/scripts/download-actionlint.bash") +RUN curl -sSL https://dot.net/v1/dotnet-install.sh | bash /dev/stdin +RUN curl -sSL https://raw.githubusercontent.com/dotnet/install-scripts/main/src/dotnet-install.sh | bash /dev/stdin +# Pinned +RUN bash <(curl --silent --show-error "https://raw.githubusercontent.com/rhysd/actionlint/7b75d16d41920ec126e6f3269db0c6f3ab613c38/scripts/download-actionlint.bash") +RUN curl -sSL https://raw.githubusercontent.com/dotnet/install-scripts/5b142a1e445a6f060d6430b661408989e9580b85/src/dotnet-install.sh | bash /dev/stdin \ No newline at end of file diff --git a/checks/raw/testdata/Dockerfile-wget-bin-sh b/checks/raw/testdata/Dockerfile-wget-bin-sh index f97b4c33b54..61988b99999 100644 --- a/checks/raw/testdata/Dockerfile-wget-bin-sh +++ b/checks/raw/testdata/Dockerfile-wget-bin-sh @@ -19,5 +19,11 @@ RUN echo hello && wget -0 - http://ifconfig.co/json | /bin/sh RUN ["echo", "hello", "&&", "wget", "-0", "-", "http://ifconfig.co/json", "|", "/bin/sh"] RUN ["sh", "-c", "\"wget -0 - http://ifconfig.co/json | /bin/sh\""] +# Unpinned +RUN wget -0 - https://raw.githubusercontent.com/dotnet/install-scripts/main/src/dotnet-install.sh | /bin/sh + +# Pinned +RUN wget -0 - https://raw.githubusercontent.com/dotnet/install-scripts/5b142a1e445a6f060d6430b661408989e9580b85/src/dotnet-install.sh | /bin/sh + FROM scratch FROM python@sha256:45b23dee08af5e43a7fea6c4cf9c25ccf269ee113168c19722f87876677c5cb2 \ No newline at end of file diff --git a/checks/raw/testdata/script-bash b/checks/raw/testdata/script-bash index 4603ac2e16e..1f04dce38f1 100644 --- a/checks/raw/testdata/script-bash +++ b/checks/raw/testdata/script-bash @@ -49,4 +49,13 @@ bash <(wget -qO- http://website.com/my-script.sh) wget http://file-with-sudo -O /tmp/file3 bash /tmp/file3 +bash <(curl --silent --show-error "https://raw.githubusercontent.com/rhysd/actionlint/main/scripts/download-actionlint.bash") +curl -sSL https://dot.net/v1/dotnet-install.sh | bash /dev/stdin +curl -sSL https://raw.githubusercontent.com/dotnet/install-scripts/main/src/dotnet-install.sh | bash /dev/stdin +RUN bash <(curl --silent --show-error "https://raw.githubusercontent.com/rhysd/actionlint/7b75d16d41920ec126e6f3269db0c6f3ab613c38/scripts/download-actionlint.bash") +RUN curl -sSL https://raw.githubusercontent.com/dotnet/install-scripts/5b142a1e445a6f060d6430b661408989e9580b85/src/dotnet-install.sh | bash /dev/stdin + +wget https://dot.net/v1/dotnet-install.sh -O /tmp/file4 +bash /tmp/file4 + date \ No newline at end of file diff --git a/checks/raw/testdata/shell-download-lines.sh b/checks/raw/testdata/shell-download-lines.sh index 6a4f934e726..2bc7bdeabd8 100644 --- a/checks/raw/testdata/shell-download-lines.sh +++ b/checks/raw/testdata/shell-download-lines.sh @@ -64,4 +64,10 @@ dotnet add package some-package dotnet add SomeProject package some-package dotnet build dotnet add package some-package -v 1.2.3 -dotnet add package some-package --version 1.2.3 \ No newline at end of file +dotnet add package some-package --version 1.2.3 + +bash <(curl --silent --show-error "https://raw.githubusercontent.com/rhysd/actionlint/main/scripts/download-actionlint.bash") +curl -sSL https://dot.net/v1/dotnet-install.sh | bash /dev/stdin +curl -sSL https://raw.githubusercontent.com/dotnet/install-scripts/main/src/dotnet-install.sh | bash /dev/stdin +RUN bash <(curl --silent --show-error "https://raw.githubusercontent.com/rhysd/actionlint/7b75d16d41920ec126e6f3269db0c6f3ab613c38/scripts/download-actionlint.bash") +RUN curl -sSL https://raw.githubusercontent.com/dotnet/install-scripts/5b142a1e445a6f060d6430b661408989e9580b85/src/dotnet-install.sh | bash /dev/stdin \ No newline at end of file