From f45007bbe9bb1fd3db9d5154d47ef898ba754d41 Mon Sep 17 00:00:00 2001 From: Simone Basso Date: Wed, 20 Mar 2024 13:15:39 +0100 Subject: [PATCH 1/2] fix(pkg/gobash/version.go): validate zip files content See https://github.com/ooni/probe-cli/security/code-scanning/12 While there, make sure we run tests using go1.22, which has been released in February while I was on leave. --- .github/workflows/go1.22.yml | 24 ++++++++++++++++++++++++ .github/workflows/gobash.yml | 1 + pkg/gobash/version.go | 12 ++++++++++++ 3 files changed, 37 insertions(+) create mode 100644 .github/workflows/go1.22.yml diff --git a/.github/workflows/go1.22.yml b/.github/workflows/go1.22.yml new file mode 100644 index 000000000..a12109f7d --- /dev/null +++ b/.github/workflows/go1.22.yml @@ -0,0 +1,24 @@ +# Runs the whole test suite using go1.22 +name: alltests-go1.22 +on: + pull_request: + push: + branches: + - "release/**" + - "fullbuild" + - "alltestsbuild" + +jobs: + test: + runs-on: ubuntu-22.04 + steps: + - uses: actions/checkout@v3 + + - uses: magnetikonline/action-golang-cache@v4 + with: + go-version: ~1.22 + cache-key-suffix: "-alltests-go1.22" + + # We cannot run buildtool tests using an unexpected version of Go because the + # tests check whether we're using the expected version of Go 😂😂😂😂. + - run: go test -race -tags shaping $(go list ./...|grep -v 'internal/cmd/buildtool') diff --git a/.github/workflows/gobash.yml b/.github/workflows/gobash.yml index 3d801d594..70dcef47c 100644 --- a/.github/workflows/gobash.yml +++ b/.github/workflows/gobash.yml @@ -28,6 +28,7 @@ jobs: - "1.19" # debian 12 "bookworm" - "1.20" - "1.21" + - "1.22" system: [ubuntu-latest, macos-latest] runs-on: "${{ matrix.system }}" steps: diff --git a/pkg/gobash/version.go b/pkg/gobash/version.go index 190a6aed9..70cd387fc 100644 --- a/pkg/gobash/version.go +++ b/pkg/gobash/version.go @@ -255,6 +255,18 @@ func unpackZip(targetDir, archiveFile string) error { defer zr.Close() for _, f := range zr.File { + // See https://github.com/ooni/probe-cli/security/code-scanning/12 + // + // The validRelPath function rejects empty paths, paths containing backslash, paths + // starting with / and paths containing ../. Additionally, according to + // src/archive/zip/reader.go, the zip specification only allows files containing + // forward slashes and considers files containing backslashes to be inscure. + // + // Therefore, by using validRelPath here, we should be able to fix the security alert. + if !validRelPath(f.Name) { + return fmt.Errorf("tar file contained invalid name %q", f.Name) + } + name := strings.TrimPrefix(f.Name, "go/") outpath := filepath.Join(targetDir, name) From 3239de925db1db289657bec05fbda303196a81f2 Mon Sep 17 00:00:00 2001 From: Simone Basso Date: Wed, 20 Mar 2024 13:20:32 +0100 Subject: [PATCH 2/2] Update pkg/gobash/version.go --- pkg/gobash/version.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/gobash/version.go b/pkg/gobash/version.go index 70cd387fc..daf375d7e 100644 --- a/pkg/gobash/version.go +++ b/pkg/gobash/version.go @@ -260,7 +260,7 @@ func unpackZip(targetDir, archiveFile string) error { // The validRelPath function rejects empty paths, paths containing backslash, paths // starting with / and paths containing ../. Additionally, according to // src/archive/zip/reader.go, the zip specification only allows files containing - // forward slashes and considers files containing backslashes to be inscure. + // forward slashes and considers files containing backslashes to be insecure. // // Therefore, by using validRelPath here, we should be able to fix the security alert. if !validRelPath(f.Name) {