Use os.Root for device file pulls (#97)

* Use os.Root for device file pulls

* Use Go 1.26.3 in workflows

Author: besendorf

.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

.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

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
-
 )

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
 			}
 		}

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
+}

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")
+	}
+}