Skip to content

Commit

Permalink
fix(pkg/gobash/version.go): validate zip files content (#1521)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
bassosimone authored Mar 20, 2024
1 parent 1b7c204 commit 9860b44
Show file tree
Hide file tree
Showing 3 changed files with 37 additions and 0 deletions.
24 changes: 24 additions & 0 deletions .github/workflows/go1.22.yml
Original file line number Diff line number Diff line change
@@ -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')
1 change: 1 addition & 0 deletions .github/workflows/gobash.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
12 changes: 12 additions & 0 deletions pkg/gobash/version.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 insecure.
//
// 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)
Expand Down

0 comments on commit 9860b44

Please sign in to comment.