Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

🐛 Trust pinned GitHub download URLs #3694

Merged
merged 6 commits into from
Nov 30, 2023
Merged

🐛 Trust pinned GitHub download URLs #3694

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
dfa618e
Trust pinned GitHub download URLs
martincostello Nov 27, 2023
e6e5e2b
Move logic to function
martincostello Nov 27, 2023
ecf0582
Fix formatting
martincostello Nov 27, 2023
6d0b106
Suppress lint warnings
martincostello Nov 27, 2023
c88c741
Address peer review
martincostello Nov 30, 2023
d493588
Fix lint warning
martincostello Nov 30, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
42 changes: 39 additions & 3 deletions checks/raw/pinned_dependencies_test.go
View file
Open in desktop
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
Open in desktop
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
Open in desktop
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 {
spencerschrock marked this conversation as resolved.
Show resolved Hide resolved
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 {
spencerschrock marked this conversation as resolved.
Show resolved Hide resolved
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
Open in desktop
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
spencerschrock marked this conversation as resolved.
Show resolved Hide resolved

FROM scratch
FROM python@sha256:45b23dee08af5e43a7fea6c4cf9c25ccf269ee113168c19722f87876677c5cb2
10 changes: 9 additions & 1 deletion checks/raw/testdata/Dockerfile-download-lines
View file
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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