fix: verify OSS objects before serving

server/media/oss/oss.go

@@ -230,6 +230,10 @@ func (oh *ossHandler) Headers(tenantID types.TenantID, method string, url *url.U
 			"Cache-Control": {oh.conf.CacheControl},
 		}, http.StatusNotModified, nil
 	}
+	objectKey, err := oh.resolveObjectKey(fdef.Location)
+	if err != nil {
+		return nil, 0, err
+	}
 
 	var redirURL string
 	switch method {
@@ -240,9 +244,9 @@ func (oh *ossHandler) Headers(tenantID types.TenantID, method string, url *url.U
 		if isAttachment, _ := strconv.ParseBool(url.Query().Get("asatt")); isAttachment {
 			options = append(options, aliyunoss.ResponseContentDisposition("attachment"))
 		}
-		redirURL, err = oh.bucket.SignURL(fdef.Location, aliyunoss.HTTPGet, int64(oh.conf.PresignTTL), options...)
+		redirURL, err = oh.bucket.SignURL(objectKey, aliyunoss.HTTPGet, int64(oh.conf.PresignTTL), options...)
 	case http.MethodHead:
-		redirURL, err = oh.bucket.SignURL(fdef.Location, aliyunoss.HTTPHead, int64(oh.conf.PresignTTL))
+		redirURL, err = oh.bucket.SignURL(objectKey, aliyunoss.HTTPHead, int64(oh.conf.PresignTTL))
 	}
 	if err != nil {
 		return nil, 0, err
@@ -284,10 +288,12 @@ func (oh *ossHandler) Upload(fdef *types.FileDef, file io.Reader) (string, int64
 
 	fdef.Location = key
 	fdef.ETag = etagFromLocation(key)
-	if meta, err := oh.bucket.GetObjectDetailedMeta(key); err == nil {
-		if etag := strings.Trim(meta.Get("ETag"), "\""); etag != "" {
-			fdef.ETag = etag
-		}
+	meta, err := oh.bucket.GetObjectDetailedMeta(key)
+	if err != nil {
+		return "", 0, err
+	}
+	if etag := strings.Trim(meta.Get("ETag"), "\""); etag != "" {
+		fdef.ETag = etag
 	}
 	return oh.conf.ServeURL + fname, rc.count, nil
 }
@@ -307,10 +313,12 @@ func (oh *ossHandler) UploadPublic(fdef *media.PublicFileDef, file io.Reader) (s
 	}
 	fdef.Location = key
 	fdef.ETag = etagFromLocation(key)
-	if meta, err := oh.bucket.GetObjectDetailedMeta(key); err == nil {
-		if etag := strings.Trim(meta.Get("ETag"), "\""); etag != "" {
-			fdef.ETag = etag
-		}
+	meta, err := oh.bucket.GetObjectDetailedMeta(key)
+	if err != nil {
+		return "", 0, err
+	}
+	if etag := strings.Trim(meta.Get("ETag"), "\""); etag != "" {
+		fdef.ETag = etag
 	}
 	return key, rc.count, nil
 }
@@ -350,6 +358,36 @@ func (oh *ossHandler) getFileRecord(tenantID types.TenantID, fid types.Uid) (*ty
 	return fd, nil
 }
 
+func (oh *ossHandler) resolveObjectKey(location string) (string, error) {
+	location = strings.TrimSpace(location)
+	if location == "" {
+		return "", types.ErrNotFound
+	}
+	if _, err := oh.bucket.GetObjectDetailedMeta(location); err == nil {
+		return location, nil
+	} else if !isOSSObjectNotFound(err) {
+		return "", err
+	}
+	if legacy := strings.TrimPrefix(location, "uploads/"); legacy != location {
+		if _, err := oh.bucket.GetObjectDetailedMeta(legacy); err == nil {
+			return legacy, nil
+		} else if !isOSSObjectNotFound(err) {
+			return "", err
+		}
+	}
+	return "", types.ErrNotFound
+}
+
+func isOSSObjectNotFound(err error) bool {
+	if err == nil {
+		return false
+	}
+	if serviceErr, ok := err.(aliyunoss.ServiceError); ok {
+		return serviceErr.StatusCode == http.StatusNotFound || serviceErr.Code == "NoSuchKey"
+	}
+	return false
+}
+
 func tenantObjectKey(tenantID types.TenantID, fileID string, createdAt time.Time) (string, error) {
 	if !tenantID.IsValid() || fileID == "" {
 		return "", types.ErrMalformed

server/media/oss/oss_tenant_test.go

@@ -1,9 +1,12 @@
 package oss
 
 import (
+	"net/http"
 	"testing"
 	"time"
 
+	aliyunoss "github.com/aliyun/aliyun-oss-go-sdk/oss"
+
 	"github.com/tinode/chat/server/store"
 	"github.com/tinode/chat/server/store/types"
 )
@@ -46,6 +49,15 @@ func TestGetIdFromUrl(t *testing.T) {
 	}
 }
 
+func TestIsOSSObjectNotFound(t *testing.T) {
+	if !isOSSObjectNotFound(aliyunoss.ServiceError{StatusCode: http.StatusNotFound, Code: "NoSuchKey"}) {
+		t.Fatal("NoSuchKey should be treated as missing object")
+	}
+	if isOSSObjectNotFound(aliyunoss.ServiceError{StatusCode: http.StatusForbidden, Code: "AccessDenied"}) {
+		t.Fatal("AccessDenied must not be treated as missing object")
+	}
+}
+
 func TestLoadSysConfig(t *testing.T) {
 	original := store.ThirdPartyConfig
 	store.ThirdPartyConfig = fakeThirdPartyConfig{