Browse Source

fix(sessionimport): harden importer against path traversal and root-walk loop

- Validate StagedFile.RelPath via safeRel: reject .. escape, absolute paths
- Replace tdata walk-up loop with direct AbsPath/RelPath suffix computation
  (old loop could spin forever at filesystem root)
- Replace hand-rolled hasSep / hasTdataPrefix with strings primitives

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
dot 2 tuần trước cách đây
mục cha
commit
a7301c627f
1 tập tin đã thay đổi với 37 bổ sung26 xóa
  1. 37 26
      internal/telegram/sessionimport/importer.go

+ 37 - 26
internal/telegram/sessionimport/importer.go

@@ -7,7 +7,9 @@ import (
 	"fmt"
 	"io"
 	"os"
+	"path"
 	"path/filepath"
+	"strings"
 
 	"github.com/gotd/td/session"
 )
@@ -77,22 +79,25 @@ func Import(ctx context.Context, sessionsDir string, stagedFiles []StagedFile) (
 
 	var metaPath, sessionPath, twoFAPath, tdataDir string
 	for _, f := range stagedFiles {
+		cleanedRel := safeRel(f.RelPath)
+		if cleanedRel == "" {
+			continue
+		}
+		rel := cleanedRel
 		switch {
-		case filepath.Ext(f.RelPath) == ".json" && !hasSep(f.RelPath):
+		case filepath.Ext(rel) == ".json" && !strings.ContainsAny(rel, "/"):
 			metaPath = f.AbsPath
-		case filepath.Ext(f.RelPath) == ".session":
+		case filepath.Ext(rel) == ".session":
 			sessionPath = f.AbsPath
-		case filepath.Base(f.RelPath) == "2fa_password.txt":
+		case filepath.Base(rel) == "2fa_password.txt":
 			twoFAPath = f.AbsPath
-		case hasTdataPrefix(f.RelPath):
-			// Remember the parent dir containing tdata/
-			dir := filepath.Dir(f.AbsPath)
-			// Walk up until we find a folder literally named "tdata".
-			for dir != "" && filepath.Base(dir) != "tdata" {
-				dir = filepath.Dir(dir)
-			}
-			if dir != "" {
-				tdataDir = dir
+		case strings.HasPrefix(rel, "tdata/"):
+			// Compute tdata dir from AbsPath + RelPath to avoid an infinite
+			// walk-up loop at filesystem root (e.g. filepath.Dir("C:\\") == "C:\\").
+			absSlash := filepath.ToSlash(f.AbsPath)
+			if strings.HasSuffix(absSlash, rel) {
+				stagingRoot := strings.TrimSuffix(absSlash, rel)
+				tdataDir = filepath.FromSlash(stagingRoot + "tdata")
 			}
 		}
 	}
@@ -171,7 +176,11 @@ func Import(ctx context.Context, sessionsDir string, stagedFiles []StagedFile) (
 	_ = os.RemoveAll(res.OriginDir)
 	if err := os.MkdirAll(res.OriginDir, 0o755); err == nil {
 		for _, f := range stagedFiles {
-			dst := filepath.Join(res.OriginDir, f.RelPath)
+			cleanedRel := safeRel(f.RelPath)
+			if cleanedRel == "" {
+				continue
+			}
+			dst := filepath.Join(res.OriginDir, cleanedRel)
 			if err := os.MkdirAll(filepath.Dir(dst), 0o755); err == nil {
 				_ = copyFile(f.AbsPath, dst)
 			}
@@ -201,21 +210,23 @@ func sanitizeTwoFA(s string) string {
 	return s
 }
 
-func hasSep(p string) bool {
-	for i := 0; i < len(p); i++ {
-		if p[i] == '/' || p[i] == '\\' {
-			return true
-		}
+// safeRel returns the cleaned RelPath if it stays inside the staging area,
+// or "" if the path tries to escape via "../", is absolute, or contains
+// suspicious tokens. Always skip the file when this returns "".
+func safeRel(p string) string {
+	// Normalize to forward slashes for consistent inspection.
+	p = filepath.ToSlash(p)
+	cleaned := path.Clean(p) // forward-slash aware
+	if cleaned == "." || cleaned == "" {
+		return ""
 	}
-	return false
-}
-
-func hasTdataPrefix(p string) bool {
-	// "tdata/..." or "tdata\..."
-	if len(p) < 6 {
-		return false
+	if strings.HasPrefix(cleaned, "../") || cleaned == ".." {
+		return ""
+	}
+	if filepath.IsAbs(cleaned) || strings.HasPrefix(cleaned, "/") {
+		return ""
 	}
-	return (p[:5] == "tdata" && (p[5] == '/' || p[5] == '\\'))
+	return cleaned
 }
 
 func nonempty(s, alt string) string {