fix storage safety edge cases
This commit is contained in:
+44
-7
@@ -41,8 +41,10 @@ var (
|
||||
ErrPathTraversal = errors.New("path traversal detected")
|
||||
// ErrInvalidPath 路径非法(空、含 NUL 等)。
|
||||
ErrInvalidPath = errors.New("invalid path")
|
||||
// ErrUploadTooLarge 上传实际字节数超过 MaxSizeBytes(P0)。客户端声明的 file.Size
|
||||
// 不可信,故除前置校验外,拷贝阶段按实际字节封顶,防止声明小体积却流式发送大 body 撑爆磁盘/OSS。
|
||||
// ErrInvalidFile 上传文件参数非法,例如 nil *multipart.FileHeader。
|
||||
ErrInvalidFile = errors.New("invalid file")
|
||||
// ErrUploadTooLarge 上传声明大小或实际字节数超过 MaxSizeBytes(P0)。客户端声明的 file.Size
|
||||
// 不可信,故除前置校验外,拷贝阶段也按实际字节封顶,防止声明小体积却流式发送大 body 撑爆磁盘/OSS。
|
||||
ErrUploadTooLarge = errors.New("upload exceeds max size")
|
||||
)
|
||||
|
||||
@@ -71,7 +73,7 @@ func resolveMaxRead(n int64) int64 {
|
||||
// 在拷贝阶段按实际字节数二次把关(P0)。
|
||||
func validateUploadSize(p config.UploadPolicy, size int64) error {
|
||||
if p.MaxSizeBytes > 0 && size > p.MaxSizeBytes {
|
||||
return fmt.Errorf("文件大小 %d 超过上限 %d: %w", size, p.MaxSizeBytes, ErrInvalidPath)
|
||||
return fmt.Errorf("文件大小 %d 超过上限 %d: %w", size, p.MaxSizeBytes, ErrUploadTooLarge)
|
||||
}
|
||||
return nil
|
||||
}
|
||||
@@ -186,6 +188,14 @@ func sanitizeObjectKey(key string) (string, error) {
|
||||
return path.Clean(key), nil
|
||||
}
|
||||
|
||||
func publicURL(baseURL, key string) string {
|
||||
cleanKey, err := sanitizeObjectKey(key)
|
||||
if err != nil {
|
||||
return ""
|
||||
}
|
||||
return strings.TrimRight(baseURL, "/") + "/" + cleanKey
|
||||
}
|
||||
|
||||
// LocalStorage 本地存储
|
||||
type LocalStorage struct {
|
||||
rootAbs string // 绝对根路径(已 Clean),前缀锚定用(C4a)
|
||||
@@ -199,7 +209,12 @@ type LocalStorage struct {
|
||||
// 安全约束:Path 指向的根目录应为框架独占目录,不与用户可控内容混用。
|
||||
// safeJoin 已防 `..` 路径穿越,但若根目录内已存在指向外部的符号链接,
|
||||
// Get 会跟随 symlink 读到根外内容——需保证攻击者无法在根目录内创建 symlink。
|
||||
// cfg 为 nil、Path 无法解析或根目录本身是 symlink 时,实例 fail-closed:
|
||||
// 后续文件操作返回 ErrStorageNotInitialized。
|
||||
func NewLocalStorage(cfg *config.LocalStorageConfig) *LocalStorage {
|
||||
if cfg == nil {
|
||||
return &LocalStorage{maxReadBytes: resolveMaxRead(0)}
|
||||
}
|
||||
// 用绝对路径作根锚定,避免相对路径 + `..` 组合绕过前缀校验(C4a)。
|
||||
var rootAbs string
|
||||
abs, err := filepath.Abs(cfg.Path)
|
||||
@@ -213,6 +228,12 @@ func NewLocalStorage(cfg *config.LocalStorageConfig) *LocalStorage {
|
||||
} else {
|
||||
rootAbs = filepath.Clean(abs)
|
||||
}
|
||||
if rootAbs != "" {
|
||||
if info, err := os.Lstat(rootAbs); err == nil && info.Mode()&os.ModeSymlink != 0 {
|
||||
logger.Error("storage: local storage root must not be a symlink", zap.String("path", rootAbs))
|
||||
rootAbs = ""
|
||||
}
|
||||
}
|
||||
return &LocalStorage{
|
||||
rootAbs: rootAbs,
|
||||
baseURL: cfg.BaseURL,
|
||||
@@ -245,6 +266,9 @@ func (s *LocalStorage) safeJoin(parts ...string) (string, error) {
|
||||
|
||||
// Upload 上传文件
|
||||
func (s *LocalStorage) Upload(file *multipart.FileHeader, subdir string) (ret string, err error) {
|
||||
if file == nil {
|
||||
return "", ErrInvalidFile
|
||||
}
|
||||
// 上传安全策略校验(大小 / 扩展名);file.Size 由 multipart 解析时填,无需打开文件(C4b)。
|
||||
if err := validateUploadSize(s.policy, file.Size); err != nil {
|
||||
return "", err
|
||||
@@ -391,7 +415,7 @@ func (s *LocalStorage) UploadFromBytes(data []byte, filename, subdir string) (re
|
||||
|
||||
// GetURL 获取文件访问 URL
|
||||
func (s *LocalStorage) GetURL(path string) string {
|
||||
return fmt.Sprintf("%s/%s", s.baseURL, path)
|
||||
return publicURL(s.baseURL, path)
|
||||
}
|
||||
|
||||
// Delete 删除文件
|
||||
@@ -462,6 +486,9 @@ type OSSStorage struct {
|
||||
|
||||
// NewOSSStorage 创建 OSS 存储实例
|
||||
func NewOSSStorage(cfg *config.OSSStorageConfig) (*OSSStorage, error) {
|
||||
if cfg == nil {
|
||||
return nil, ErrStorageNotInitialized
|
||||
}
|
||||
client, err := oss.New(cfg.Endpoint, cfg.AccessKeyID, cfg.AccessKeySecret)
|
||||
if err != nil {
|
||||
return nil, fmt.Errorf("创建 OSS 客户端失败: %w", err)
|
||||
@@ -485,6 +512,9 @@ func NewOSSStorage(cfg *config.OSSStorageConfig) (*OSSStorage, error) {
|
||||
|
||||
// Upload 上传文件到 OSS
|
||||
func (s *OSSStorage) Upload(file *multipart.FileHeader, subdir string) (string, error) {
|
||||
if file == nil {
|
||||
return "", ErrInvalidFile
|
||||
}
|
||||
// 上传安全策略校验(C4b)
|
||||
if err := validateUploadSize(s.policy, file.Size); err != nil {
|
||||
return "", err
|
||||
@@ -576,10 +606,14 @@ func (s *OSSStorage) UploadFromBytes(data []byte, filename, subdir string) (stri
|
||||
|
||||
// GetURL 获取文件访问 URL
|
||||
func (s *OSSStorage) GetURL(path string) string {
|
||||
if s.baseURL != "" {
|
||||
return fmt.Sprintf("%s/%s", s.baseURL, path)
|
||||
cleanKey, err := sanitizeObjectKey(path)
|
||||
if err != nil {
|
||||
return ""
|
||||
}
|
||||
return fmt.Sprintf("https://%s.%s/%s", s.bucketName, s.endpoint, path)
|
||||
if s.baseURL != "" {
|
||||
return strings.TrimRight(s.baseURL, "/") + "/" + cleanKey
|
||||
}
|
||||
return fmt.Sprintf("https://%s.%s/%s", s.bucketName, s.endpoint, cleanKey)
|
||||
}
|
||||
|
||||
// GetSignedURL 获取带签名的临时访问 URL(用于私有文件)
|
||||
@@ -674,6 +708,9 @@ func SetDefaultStorageManager(m *StorageManager) {
|
||||
|
||||
// Init 初始化存储
|
||||
func (m *StorageManager) Init(cfg *config.StorageConfig) error {
|
||||
if cfg == nil {
|
||||
return ErrStorageNotInitialized
|
||||
}
|
||||
m.mu.Lock()
|
||||
defer m.mu.Unlock()
|
||||
|
||||
|
||||
@@ -1,6 +1,7 @@
|
||||
package storage
|
||||
|
||||
import (
|
||||
"errors"
|
||||
"strings"
|
||||
"testing"
|
||||
)
|
||||
@@ -64,3 +65,28 @@ func TestResolveMaxRead(t *testing.T) {
|
||||
t.Errorf("resolveMaxRead(2048) = %d, want 2048", got)
|
||||
}
|
||||
}
|
||||
|
||||
func TestNewOSSStorageNilConfig(t *testing.T) {
|
||||
if _, err := NewOSSStorage(nil); !errors.Is(err, ErrStorageNotInitialized) {
|
||||
t.Fatalf("NewOSSStorage(nil) err = %v, want ErrStorageNotInitialized", err)
|
||||
}
|
||||
}
|
||||
|
||||
func TestOSSStorageGetURLSanitizesPath(t *testing.T) {
|
||||
s := &OSSStorage{
|
||||
baseURL: "https://cdn.example.com/",
|
||||
bucketName: "bucket",
|
||||
endpoint: "oss.example.com",
|
||||
}
|
||||
if got := s.GetURL("docs//a.txt"); got != "https://cdn.example.com/docs/a.txt" {
|
||||
t.Fatalf("GetURL clean path = %q", got)
|
||||
}
|
||||
if got := s.GetURL("../secret.txt"); got != "" {
|
||||
t.Fatalf("GetURL traversal = %q, want empty", got)
|
||||
}
|
||||
|
||||
s.baseURL = ""
|
||||
if got := s.GetURL("docs/a.txt"); got != "https://bucket.oss.example.com/docs/a.txt" {
|
||||
t.Fatalf("GetURL endpoint fallback = %q", got)
|
||||
}
|
||||
}
|
||||
|
||||
@@ -196,8 +196,8 @@ func TestLocalStorageGetReadLimit(t *testing.T) {
|
||||
func TestLocalStorageUploadSizeLimit(t *testing.T) {
|
||||
s := newLocalStorageWithPolicy(t, config.UploadPolicy{MaxSizeBytes: 5}, 0)
|
||||
fh := makeFileHeader(t, "file", "big.txt", "text/plain", []byte("0123456789")) // 10 字节
|
||||
if _, err := s.Upload(fh, "docs"); !errors.Is(err, storage.ErrInvalidPath) {
|
||||
t.Errorf("Upload over size err = %v, want ErrInvalidPath", err)
|
||||
if _, err := s.Upload(fh, "docs"); !errors.Is(err, storage.ErrUploadTooLarge) {
|
||||
t.Errorf("Upload over size err = %v, want ErrUploadTooLarge", err)
|
||||
}
|
||||
}
|
||||
|
||||
@@ -239,14 +239,14 @@ func TestLocalStorageUploadMIMESniff(t *testing.T) {
|
||||
// 回归 C4b:UploadFromBytes 同样受策略约束(大小 / 扩展名 / MIME)。
|
||||
func TestLocalStorageUploadFromBytesPolicy(t *testing.T) {
|
||||
s := newLocalStorageWithPolicy(t, config.UploadPolicy{
|
||||
MaxSizeBytes: 100,
|
||||
AllowedExts: []string{".txt"},
|
||||
AllowedMIMEs: []string{"text/plain"},
|
||||
MaxSizeBytes: 100,
|
||||
AllowedExts: []string{".txt"},
|
||||
AllowedMIMEs: []string{"text/plain"},
|
||||
}, 0)
|
||||
|
||||
// 超限
|
||||
if _, err := s.UploadFromBytes(make([]byte, 200), "ok.txt", "docs"); !errors.Is(err, storage.ErrInvalidPath) {
|
||||
t.Errorf("UploadFromBytes over size err = %v, want ErrInvalidPath", err)
|
||||
if _, err := s.UploadFromBytes(make([]byte, 200), "ok.txt", "docs"); !errors.Is(err, storage.ErrUploadTooLarge) {
|
||||
t.Errorf("UploadFromBytes over size err = %v, want ErrUploadTooLarge", err)
|
||||
}
|
||||
// 扩展名不符
|
||||
if _, err := s.UploadFromBytes([]byte("hi"), "evil.php", "docs"); !errors.Is(err, storage.ErrInvalidPath) {
|
||||
@@ -295,3 +295,50 @@ func TestLocalStorageZeroPolicyAllowsAll(t *testing.T) {
|
||||
t.Errorf("Upload with zero policy err = %v (must remain unrestricted for backward compat)", err)
|
||||
}
|
||||
}
|
||||
|
||||
func TestLocalStorageNilConfigFailsClosed(t *testing.T) {
|
||||
s := storage.NewLocalStorage(nil)
|
||||
if _, err := s.Get("x.txt"); !errors.Is(err, storage.ErrStorageNotInitialized) {
|
||||
t.Fatalf("Get with nil config err = %v, want ErrStorageNotInitialized", err)
|
||||
}
|
||||
}
|
||||
|
||||
func TestStorageInitNilConfigNoPanic(t *testing.T) {
|
||||
if err := storage.NewStorageManager().Init(nil); !errors.Is(err, storage.ErrStorageNotInitialized) {
|
||||
t.Fatalf("Init(nil) err = %v, want ErrStorageNotInitialized", err)
|
||||
}
|
||||
}
|
||||
|
||||
func TestLocalStorageUploadNilFile(t *testing.T) {
|
||||
s := newLocalStorageWithPolicy(t, config.UploadPolicy{}, 0)
|
||||
if _, err := s.Upload(nil, "docs"); !errors.Is(err, storage.ErrInvalidFile) {
|
||||
t.Fatalf("Upload(nil) err = %v, want ErrInvalidFile", err)
|
||||
}
|
||||
}
|
||||
|
||||
func TestLocalStorageRejectsSymlinkRoot(t *testing.T) {
|
||||
parent := t.TempDir()
|
||||
target := filepath.Join(parent, "target")
|
||||
if err := os.Mkdir(target, 0755); err != nil {
|
||||
t.Fatalf("mkdir target: %v", err)
|
||||
}
|
||||
link := filepath.Join(parent, "link")
|
||||
if err := os.Symlink(target, link); err != nil {
|
||||
t.Skipf("symlink not available on this system: %v", err)
|
||||
}
|
||||
|
||||
s := storage.NewLocalStorage(&config.LocalStorageConfig{Path: link, BaseURL: "http://localhost/uploads"})
|
||||
if _, err := s.Get("x.txt"); !errors.Is(err, storage.ErrStorageNotInitialized) {
|
||||
t.Fatalf("Get with symlink root err = %v, want ErrStorageNotInitialized", err)
|
||||
}
|
||||
}
|
||||
|
||||
func TestLocalStorageGetURLSanitizesPath(t *testing.T) {
|
||||
s := newLocalStorageWithPolicy(t, config.UploadPolicy{}, 0)
|
||||
if got := s.GetURL("docs//a.txt"); got != "http://localhost/uploads/docs/a.txt" {
|
||||
t.Fatalf("GetURL clean path = %q", got)
|
||||
}
|
||||
if got := s.GetURL("../secret.txt"); got != "" {
|
||||
t.Fatalf("GetURL traversal = %q, want empty", got)
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user