Skip to content

Commit

Permalink
🐛 Trust pinned GitHub download URLs (ossf#3694)
Browse files Browse the repository at this point in the history 
* 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 ossf#3339.
Signed-off-by: martincostello <martin@martincostello.com>

* Move logic to function

- Add `hasUnpinnedURLs` function.
- Add test cases for different URLs.
Signed-off-by: martincostello <martin@martincostello.com>

* Fix formatting

Appease the linter.
Signed-off-by: martincostello <martin@martincostello.com>

* Suppress lint warnings

Suppress warning on three long URLs.
Signed-off-by: martincostello <martin@martincostello.com>

* Address peer review

Address peer review feedback.
Signed-off-by: martincostello <martin@martincostello.com>

* Fix lint warning

Fix lint warning.
Signed-off-by: martincostello <martin@martincostello.com>
Signed-off-by: Allen Shearin <allen.p.shearin@gmail.com>
  • Loading branch information
@martincostello @ashearin
martincostello authored and ashearin committed Dec 4, 2023
1 parent dcbfa3c commit 794b7ef
Show file tree
Hide file tree
Showing 8 changed files with 240 additions and 5 deletions.
42 changes: 39 additions & 3 deletions checks/raw/pinned_dependencies_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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,
},
},
},
{
Expand Down Expand Up @@ -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,
},
},
},
}
Expand Down Expand Up @@ -1079,7 +1115,7 @@ func TestDockerfileScriptDownload(t *testing.T) {
{
name: "curl | sh",
filename: "./testdata/Dockerfile-curl-sh",
unpinned: 4,
unpinned: 5,
},
{
name: "empty file",
Expand All @@ -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",
Expand Down Expand Up @@ -1262,7 +1298,7 @@ func TestShellScriptDownload(t *testing.T) {
{
name: "bash script",
filename: "./testdata/script-bash",
unpinned: 7,
unpinned: 11,
},
{
name: "sh script 2",
Expand Down
36 changes: 36 additions & 0 deletions checks/raw/shell_download_validate.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down Expand Up @@ -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,
) {
Expand Down Expand Up @@ -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,
Expand Down
128 changes: 128 additions & 0 deletions checks/raw/shell_download_validate_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
})
}
}
6 changes: 6 additions & 0 deletions checks/raw/testdata/Dockerfile-curl-sh
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
10 changes: 9 additions & 1 deletion checks/raw/testdata/Dockerfile-download-lines
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
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
6 changes: 6 additions & 0 deletions checks/raw/testdata/Dockerfile-wget-bin-sh
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
9 changes: 9 additions & 0 deletions checks/raw/testdata/script-bash
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
8 changes: 7 additions & 1 deletion checks/raw/testdata/shell-download-lines.sh
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
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

0 comments on commit 794b7ef

Please sign in to comment.