Skip to content

Commit 9cc5dd9

Browse files
SuyunmengILoveScratch2j2rong4cn
committed
fix(offline_download): block SimpleHttp temp file path traversal via strict filename sanitization
* fix(offline_download): prevent path traversal * fix(SimpleHttp): improve filename validation * fix(offline_download): harden SimpleHttp filename and temp path checks * simplify filename validation --------- Co-authored-by: ILoveScratch2 <ilovescratch@foxmail.com> Co-authored-by: j2rong4cn <j2rong@qq.com>
1 parent 98c32d3 commit 9cc5dd9

2 files changed

Lines changed: 32 additions & 6 deletions

File tree

internal/offline_download/http/client.go

Lines changed: 11 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,6 @@ import (
55
"math/rand/v2"
66
"net/http"
77
"os"
8-
"path"
98
"path/filepath"
109
"strings"
1110
"time"
@@ -73,11 +72,11 @@ func (s SimpleHttp) Run(task *tool.DownloadTask) error {
7372
}
7473
filename, err := parseFilenameFromContentDisposition(resp.Header.Get("Content-Disposition"))
7574
if err != nil {
76-
filename = path.Base(resp.Request.URL.Path)
75+
filename, err = sanitizeFilename(resp.Request.URL.Path)
7776
}
78-
filename = strings.Trim(filename, "/")
79-
if len(filename) == 0 {
80-
filename = fmt.Sprintf("%s-%d-%x", strings.ReplaceAll(req.URL.Host, ".", "_"), time.Now().UnixMilli(), rand.Uint32())
77+
if err != nil {
78+
filename = strings.ReplaceAll(req.URL.Host, ":", "_")
79+
filename = fmt.Sprintf("%s-%d-%x", filename, time.Now().UnixMilli(), rand.Uint32())
8180
}
8281
fileSize := resp.ContentLength
8382
if streamPut {
@@ -91,8 +90,14 @@ func (s SimpleHttp) Run(task *tool.DownloadTask) error {
9190
}
9291
task.SetTotalBytes(fileSize)
9392
// save to temp dir
94-
_ = os.MkdirAll(task.TempDir, os.ModePerm)
93+
if err := os.MkdirAll(task.TempDir, os.ModePerm); err != nil {
94+
return err
95+
}
9596
filePath := filepath.Join(task.TempDir, filename)
97+
cleanTempDir := filepath.Clean(task.TempDir) + string(filepath.Separator)
98+
if !strings.HasPrefix(filepath.Clean(filePath)+string(filepath.Separator), cleanTempDir) {
99+
return fmt.Errorf("filename illegal")
100+
}
96101
file, err := os.Create(filePath)
97102
if err != nil {
98103
return err

internal/offline_download/http/util.go

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,8 +3,22 @@ package http
33
import (
44
"fmt"
55
"mime"
6+
"path/filepath"
7+
"strings"
68
)
79

10+
func sanitizeFilename(raw string) (string, error) {
11+
raw = strings.TrimSpace(raw)
12+
sep := string(filepath.Separator)
13+
raw = strings.ReplaceAll(raw, "/", sep)
14+
raw = strings.ReplaceAll(raw, "\\", sep)
15+
filename := filepath.Base(filepath.Clean(raw))
16+
if filename == "." || filename == sep || !filepath.IsLocal(filename) {
17+
return "", fmt.Errorf("invalid filename: %q", raw)
18+
}
19+
return filename, nil
20+
}
21+
822
func parseFilenameFromContentDisposition(contentDisposition string) (string, error) {
923
if contentDisposition == "" {
1024
return "", fmt.Errorf("Content-Disposition is empty")
@@ -14,8 +28,15 @@ func parseFilenameFromContentDisposition(contentDisposition string) (string, err
1428
return "", err
1529
}
1630
filename := params["filename"]
31+
if filename == "" {
32+
filename = params["filename*"]
33+
}
1734
if filename == "" {
1835
return "", fmt.Errorf("filename not found in Content-Disposition: [%s]", contentDisposition)
1936
}
37+
filename, err = sanitizeFilename(filename)
38+
if err != nil {
39+
return "", fmt.Errorf("invalid filename in Content-Disposition: [%s]", contentDisposition)
40+
}
2041
return filename, nil
2142
}

0 commit comments

Comments
 (0)