diff --git a/.github/workflows/release.yaml b/.github/workflows/release.yaml index a6745e9..ad9157f 100644 --- a/.github/workflows/release.yaml +++ b/.github/workflows/release.yaml @@ -21,7 +21,7 @@ jobs: - name: Set up Go uses: actions/setup-go@v5 with: - go-version: "1.23" + go-version: "1.26.3" cache: true - name: Install cross-compilation tools diff --git a/.github/workflows/staticcheck.yaml b/.github/workflows/staticcheck.yaml index e77e332..25d6ed4 100644 --- a/.github/workflows/staticcheck.yaml +++ b/.github/workflows/staticcheck.yaml @@ -11,7 +11,7 @@ jobs: fetch-depth: 1 - uses: actions/setup-go@v5 with: - go-version: "1.23" + go-version: "1.26.3" cache: true - name: download assets run: make download diff --git a/go.mod b/go.mod index 8caf519..0eeb431 100644 --- a/go.mod +++ b/go.mod @@ -1,8 +1,6 @@ module github.com/mvt-project/androidqf -go 1.23.0 - -toolchain go1.24.5 +go 1.26.3 require ( filippo.io/age v1.2.1 @@ -21,5 +19,4 @@ require ( github.com/xo/terminfo v0.0.0-20220910002029-abceb7e1c41e // indirect golang.org/x/crypto v0.40.0 // indirect golang.org/x/sys v0.34.0 // indirect - ) diff --git a/modules/intrusion_logs.go b/modules/intrusion_logs.go index d0c2219..a6dca5f 100644 --- a/modules/intrusion_logs.go +++ b/modules/intrusion_logs.go @@ -10,6 +10,7 @@ import ( "fmt" "os" "os/signal" + "path" "path/filepath" "strings" "time" @@ -249,16 +250,32 @@ func (m *IL) waitForNewFiles( } func (m *IL) pullAll(acq *acquisition.Acquisition, deviceFiles []string) error { + streaming := acq.StreamingMode && acq.EncryptedWriter != nil + var localRoot *os.Root + var puller *acquisition.StreamingPuller + if !streaming { + var err error + localRoot, err = os.OpenRoot(m.ILPath) + if err != nil { + return fmt.Errorf("failed to open intrusion logs output root: %v", err) + } + defer localRoot.Close() + puller = acquisition.NewStreamingPuller(adb.Client.ExePath, adb.Client.Serial, 100) + } + for _, file := range deviceFiles { if file == m.DirOnDevice { continue } - rel := strings.TrimPrefix(file, m.DirOnDevice) - rel = strings.TrimPrefix(rel, "/") // optional safety if DirOnDevice lacks trailing / + rel, err := relativeDeviceChild(m.DirOnDevice, file) + if err != nil { + log.Errorf("Skipping IL file with unsafe path %s: %v\n", file, err) + continue + } - if acq.StreamingMode && acq.EncryptedWriter != nil { - zipPath := fmt.Sprintf("intrusion_logs/%s", rel) + if streaming { + zipPath := path.Join("intrusion_logs", rel) writer, err := acq.EncryptedWriter.CreateFile(zipPath) if err != nil { @@ -274,16 +291,8 @@ func (m *IL) pullAll(acq *acquisition.Acquisition, deviceFiles []string) error { log.Debugf("Streamed IL file %s directly to encrypted archive as %s", file, zipPath) } else { - destPath := filepath.Join(m.ILPath, rel) - - if err := os.MkdirAll(filepath.Dir(destPath), 0o755); err != nil { - log.Errorf("Failed to create folders for IL file %s: %v\n", destPath, err) - continue - } - - out, err := adb.Client.Pull(file, destPath) - if err != nil { - log.Errorf("Failed to pull IL file %s: %s\n", file, strings.TrimSpace(out)) + if err := streamDeviceChildToRoot(localRoot, puller, rel, file); err != nil { + log.Errorf("Failed to pull IL file %s: %v\n", file, err) continue } } diff --git a/modules/paths.go b/modules/paths.go new file mode 100644 index 0000000..e32cf81 --- /dev/null +++ b/modules/paths.go @@ -0,0 +1,85 @@ +// Copyright (c) 2021-2026 Claudio Guarnieri. +// Use of this software is governed by the MVT License 1.1 that can be found at +// https://license.mvt.re/1.1/ + +package modules + +import ( + "fmt" + "io" + "os" + "path" + "path/filepath" + "strings" +) + +type pullToWriter interface { + PullToWriter(remotePath string, writer io.Writer) error +} + +func relativeDeviceChild(deviceRoot, devicePath string) (string, error) { + if deviceRoot == "" { + return "", fmt.Errorf("device root cannot be empty") + } + if strings.ContainsRune(devicePath, 0) { + return "", fmt.Errorf("unsafe device path %q", devicePath) + } + + root := path.Clean(deviceRoot) + child := path.Clean(devicePath) + if child == root { + return "", fmt.Errorf("device path %q is the root path %q", devicePath, deviceRoot) + } + + rootPrefix := root + if !strings.HasSuffix(rootPrefix, "/") { + rootPrefix += "/" + } + if !strings.HasPrefix(child, rootPrefix) { + return "", fmt.Errorf("device path %q is outside %q", devicePath, deviceRoot) + } + + rel := strings.TrimPrefix(child, rootPrefix) + localRel := filepath.FromSlash(rel) + if !filepath.IsLocal(localRel) { + return "", fmt.Errorf("unsafe device path %q", devicePath) + } + + return rel, nil +} + +func createRootFile(root *os.Root, rel string) (*os.File, error) { + localRel := filepath.FromSlash(rel) + if !filepath.IsLocal(localRel) { + return nil, fmt.Errorf("unsafe local path %q", rel) + } + + if err := root.MkdirAll(filepath.Dir(localRel), 0o755); err != nil { + return nil, fmt.Errorf("failed to create destination folders for %q: %v", rel, err) + } + + file, err := root.OpenFile(localRel, os.O_WRONLY|os.O_CREATE|os.O_TRUNC, 0o644) + if err != nil { + return nil, fmt.Errorf("failed to create destination file %q: %v", rel, err) + } + + return file, nil +} + +func streamDeviceChildToRoot(root *os.Root, puller pullToWriter, rel, devicePath string) error { + file, err := createRootFile(root, rel) + if err != nil { + return err + } + defer file.Close() + + if err := puller.PullToWriter(devicePath, file); err != nil { + return err + } + + if err := file.Sync(); err != nil { + return fmt.Errorf("failed to sync destination file %q: %v", rel, err) + } + + return nil +} diff --git a/modules/paths_test.go b/modules/paths_test.go new file mode 100644 index 0000000..ad5a83c --- /dev/null +++ b/modules/paths_test.go @@ -0,0 +1,151 @@ +// Copyright (c) 2021-2026 Claudio Guarnieri. +// Use of this software is governed by the MVT License 1.1 that can be found at +// https://license.mvt.re/1.1/ + +package modules + +import ( + "errors" + "os" + "path/filepath" + "runtime" + "testing" +) + +func TestRelativeDeviceChild(t *testing.T) { + tests := []struct { + name string + deviceRoot string + devicePath string + want string + wantErr bool + }{ + { + name: "child with trailing slash root", + deviceRoot: "/sdcard/Download/Intrusion Logging/", + devicePath: "/sdcard/Download/Intrusion Logging/logs/file.txt", + want: "logs/file.txt", + }, + { + name: "child without trailing slash root", + deviceRoot: "/data/local/tmp", + devicePath: "/data/local/tmp/file.txt", + want: "file.txt", + }, + { + name: "sibling prefix rejected", + deviceRoot: "/data/local/tmp", + devicePath: "/data/local/tmp-evil/file.txt", + wantErr: true, + }, + { + name: "parent traversal rejected", + deviceRoot: "/data/local/tmp", + devicePath: "/data/local/tmp/../../../host/path", + wantErr: true, + }, + { + name: "cleaned child traversal rejected", + deviceRoot: "/sdcard/Download/Intrusion Logging/", + devicePath: "/sdcard/Download/Intrusion Logging/../Other/file.txt", + wantErr: true, + }, + { + name: "non child rejected", + deviceRoot: "/sdcard/Download/Intrusion Logging/", + devicePath: "/sdcard/Download/Other/file.txt", + wantErr: true, + }, + { + name: "root rejected", + deviceRoot: "/data/local/tmp/", + devicePath: "/data/local/tmp", + wantErr: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got, err := relativeDeviceChild(tt.deviceRoot, tt.devicePath) + if tt.wantErr { + if err == nil { + t.Fatalf("relativeDeviceChild() error = nil, want error") + } + return + } + if err != nil { + t.Fatalf("relativeDeviceChild() error = %v", err) + } + if got != tt.want { + t.Fatalf("relativeDeviceChild() = %q, want %q", got, tt.want) + } + }) + } +} + +func TestCreateRootFile(t *testing.T) { + rootDir := t.TempDir() + root, err := os.OpenRoot(rootDir) + if err != nil { + t.Fatalf("OpenRoot() error = %v", err) + } + defer root.Close() + + file, err := createRootFile(root, "nested/file.txt") + if err != nil { + t.Fatalf("createRootFile() error = %v", err) + } + if _, err := file.WriteString("ok"); err != nil { + t.Fatalf("WriteString() error = %v", err) + } + if err := file.Close(); err != nil { + t.Fatalf("Close() error = %v", err) + } + + got, err := os.ReadFile(filepath.Join(rootDir, "nested", "file.txt")) + if err != nil { + t.Fatalf("ReadFile() error = %v", err) + } + if string(got) != "ok" { + t.Fatalf("created file content = %q, want %q", got, "ok") + } + + file, err = createRootFile(root, "file.txt") + if err != nil { + t.Fatalf("createRootFile() root file error = %v", err) + } + if err := file.Close(); err != nil { + t.Fatalf("Close() root file error = %v", err) + } + + if file, err := createRootFile(root, "../escape"); err == nil { + file.Close() + t.Fatal("createRootFile() error = nil, want lexical traversal rejection") + } +} + +func TestCreateRootFileRejectsSymlinkEscape(t *testing.T) { + if runtime.GOOS == "windows" { + t.Skip("symlink creation requires extra privileges on Windows") + } + + rootDir := t.TempDir() + outsideDir := t.TempDir() + if err := os.Symlink(outsideDir, filepath.Join(rootDir, "escape")); err != nil { + if errors.Is(err, os.ErrPermission) { + t.Skipf("symlink creation not permitted: %v", err) + } + t.Fatalf("Symlink() error = %v", err) + } + + root, err := os.OpenRoot(rootDir) + if err != nil { + t.Fatalf("OpenRoot() error = %v", err) + } + defer root.Close() + + if file, err := createRootFile(root, "escape/file.txt"); err == nil { + file.Close() + t.Fatal("createRootFile() error = nil, want symlink escape rejection") + } +} diff --git a/modules/temp.go b/modules/temp.go index cf95579..9de493a 100644 --- a/modules/temp.go +++ b/modules/temp.go @@ -7,8 +7,8 @@ package modules import ( "fmt" "os" + "path" "path/filepath" - "strings" "github.com/botherder/go-savetime/text" "github.com/mvt-project/androidqf/acquisition" @@ -47,6 +47,19 @@ func (t *Temp) InitStorage(storagePath string) error { func (t *Temp) Run(acq *acquisition.Acquisition, fast bool) error { log.Info("Collecting files in tmp folder...") + streaming := acq.StreamingMode && acq.EncryptedWriter != nil + var localRoot *os.Root + var puller *acquisition.StreamingPuller + if !streaming { + var err error + localRoot, err = os.OpenRoot(t.TempPath) + if err != nil { + return fmt.Errorf("failed to open tmp output root: %v", err) + } + defer localRoot.Close() + puller = acquisition.NewStreamingPuller(adb.Client.ExePath, adb.Client.Serial, 100) + } + // TODO: Also check default tmp folders tmpFiles, err := adb.Client.ListFiles(acq.TmpDir, true) if err != nil { @@ -58,9 +71,15 @@ func (t *Temp) Run(acq *acquisition.Acquisition, fast bool) error { continue } - if acq.StreamingMode && acq.EncryptedWriter != nil { + rel, err := relativeDeviceChild(acq.TmpDir, file) + if err != nil { + log.Errorf("Skipping temp file with unsafe path %s: %v\n", file, err) + continue + } + + if streaming { // Streaming mode: stream directly from ADB to encrypted zip without temp files - zipPath := fmt.Sprintf("tmp/%s", strings.TrimPrefix(file, acq.TmpDir)) + zipPath := path.Join("tmp", rel) // Create zip entry writer writer, err := acq.EncryptedWriter.CreateFile(zipPath) @@ -78,14 +97,10 @@ func (t *Temp) Run(acq *acquisition.Acquisition, fast bool) error { log.Debugf("Streamed temp file %s directly to encrypted archive as %s", file, zipPath) } else { - // Traditional mode: pull directly to local storage - dest_path := filepath.Join(t.TempPath, - strings.TrimPrefix(file, acq.TmpDir)) - - out, err := adb.Client.Pull(file, dest_path) - if err != nil { - if !text.ContainsNoCase(out, "Permission denied") { - log.Errorf("Failed to pull temp file %s: %s\n", file, strings.TrimSpace(out)) + // Traditional mode: stream into a file opened relative to t.TempPath. + if err := streamDeviceChildToRoot(localRoot, puller, rel, file); err != nil { + if !text.ContainsNoCase(err.Error(), "Permission denied") { + log.Errorf("Failed to pull temp file %s: %v\n", file, err) } continue }