diff --git a/server/db/mysql/adapter.go b/server/db/mysql/adapter.go index 89e049c..9de4963 100644 --- a/server/db/mysql/adapter.go +++ b/server/db/mysql/adapter.go @@ -72,6 +72,27 @@ func quoteIdent(name string) string { return "`" + strings.ReplaceAll(name, "`", "``") + "`" } +func parseFileUID(fid string) t.Uid { + name := strings.TrimSpace(fid) + id := t.ParseUid(name) + if !id.IsZero() { + return id + } + if dot := strings.LastIndexByte(name, '.'); dot > 0 { + ext := name[dot+1:] + if len(ext) == 0 || len(ext) > 16 { + return t.ZeroUid + } + for _, r := range ext { + if (r < 'A' || r > 'Z') && (r < 'a' || r > 'z') && (r < '0' || r > '9') { + return t.ZeroUid + } + } + return t.ParseUid(name[:dot]) + } + return t.ZeroUid +} + type configType struct { // DB connection settings. // See https://pkg.go.dev/github.com/go-sql-driver/mysql#Config @@ -4994,7 +5015,7 @@ func (a *adapter) FileFinishUpload(fd *t.FileDef, success bool, size int64) (*t. // FileGet fetches a record of a specific file func (a *adapter) FileGet(fid string) (*t.FileDef, error) { - id := t.ParseUid(fid) + id := parseFileUID(fid) if id.IsZero() { return nil, t.ErrMalformed } @@ -5188,7 +5209,7 @@ func (a *adapter) FileUserHasAccess(fid string, uid t.Uid) (bool, error) { if uid.IsZero() { return false, t.ErrMalformed } - fileID := t.ParseUid(fid) + fileID := parseFileUID(fid) if fileID.IsZero() { return false, t.ErrMalformed } diff --git a/server/db/mysql/adapter_fileid_test.go b/server/db/mysql/adapter_fileid_test.go new file mode 100644 index 0000000..5834821 --- /dev/null +++ b/server/db/mysql/adapter_fileid_test.go @@ -0,0 +1,26 @@ +//go:build mysql +// +build mysql + +package mysql + +import ( + "testing" + + "github.com/tinode/chat/server/store/types" +) + +func TestParseFileUIDStripsPublicExtension(t *testing.T) { + uid := types.ParseUid("scH0N8guCHk") + if uid.IsZero() { + t.Fatal("test UID failed to parse") + } + if got := parseFileUID("scH0N8guCHk.bin"); got != uid { + t.Fatalf("parseFileUID()=%q, want %q", got.String(), uid.String()) + } + if got := parseFileUID("scH0N8guCHk.jpg"); got != uid { + t.Fatalf("parseFileUID()=%q, want %q", got.String(), uid.String()) + } + if got := parseFileUID("scH0N8guCHk"); got != uid { + t.Fatalf("parseFileUID()=%q, want %q", got.String(), uid.String()) + } +} diff --git a/server/db/mysql/tenant_adapter.go b/server/db/mysql/tenant_adapter.go index df8a3ae..b82a5fb 100644 --- a/server/db/mysql/tenant_adapter.go +++ b/server/db/mysql/tenant_adapter.go @@ -7678,7 +7678,7 @@ func (a *adapter) TenantFileGet(id t.TenantID, fid string) (*t.FileDef, error) { file, err := a.FileGet(fid) return stampTenantFile(id, file), err } - fileID := t.ParseUid(fid) + fileID := parseFileUID(fid) if fileID.IsZero() { return nil, t.ErrMalformed } @@ -7859,7 +7859,7 @@ func (a *adapter) TenantFileUserHasAccess(id t.TenantID, fid string, uid t.Uid) if a.tenantScopedDB { return a.FileUserHasAccess(fid, uid) } - fileID := t.ParseUid(fid) + fileID := parseFileUID(fid) if fileID.IsZero() { return false, t.ErrMalformed } diff --git a/server/hdl_files.go b/server/hdl_files.go index b38b3c2..a06d2e3 100644 --- a/server/hdl_files.go +++ b/server/hdl_files.go @@ -113,6 +113,11 @@ func largeFileServeHTTP(wrt http.ResponseWriter, req *http.Request) { return } + if err = authorizeLargeFileRequestURL(principal.TenantID, principal.UID, req.URL.String()); err != nil { + writeHttpResponse(decodeStoreError(err, "", now, nil), err) + return + } + // Check if media handler redirects or adds headers. headers, statusCode, err := mh.Headers(principal.TenantID, req.Method, req.URL, req.Header, true) if err != nil { @@ -517,6 +522,21 @@ func (*grpcNodeServer) LargeFileServe(req *pbx.FileDownReq, stream pbx.Node_Larg return err } +func authorizeLargeFileRequestURL(tenantID types.TenantID, uid types.Uid, rawURL string) error { + if store.Store == nil { + return types.ErrUnsupported + } + fid := store.Store.GetMediaHandler().GetIdFromUrl(rawURL) + if fid.IsZero() { + return types.ErrNotFound + } + fd, err := store.Files.Get(tenantID, fid.String()) + if err != nil { + return err + } + return authorizeLargeFileDownload(tenantID, uid, rawURL, fd) +} + func authorizeLargeFileDownload(tenantID types.TenantID, uid types.Uid, rawURL string, fd *types.FileDef) error { if fd == nil { return types.ErrNotFound diff --git a/server/hdl_files_test.go b/server/hdl_files_test.go index a2abea1..8621038 100644 --- a/server/hdl_files_test.go +++ b/server/hdl_files_test.go @@ -22,12 +22,15 @@ import ( ) type testLargeFileMediaHandler struct { - def *types.FileDef - data string + def *types.FileDef + data string + fid types.Uid + headersCalled bool } func (h *testLargeFileMediaHandler) Init(string) error { return nil } func (h *testLargeFileMediaHandler) Headers(types.TenantID, string, *url.URL, http.Header, bool) (http.Header, int, error) { + h.headersCalled = true return nil, 0, nil } func (h *testLargeFileMediaHandler) Upload(def *types.FileDef, file io.Reader) (string, int64, error) { @@ -43,7 +46,7 @@ func (h *testLargeFileMediaHandler) Download(types.TenantID, string) (*types.Fil } func (h *testLargeFileMediaHandler) Delete([]string) error { return nil } func (h *testLargeFileMediaHandler) GetIdFromUrl(string) types.Uid { - return types.ZeroUid + return h.fid } func TestAuthFileRequestFromSessionRequiresTenant(t *testing.T) { @@ -144,6 +147,43 @@ func TestAuthorizeLargeFileDownloadRejectsCrossTenant(t *testing.T) { } } +func TestAuthorizeLargeFileRequestURLRequiresAccessBeforeRedirect(t *testing.T) { + ctrl := gomock.NewController(t) + defer ctrl.Finish() + + oldStore := store.Store + oldFiles := store.Files + defer func() { + store.Store = oldStore + store.Files = oldFiles + }() + + fid := types.Uid(100) + handler := &testLargeFileMediaHandler{fid: fid} + mockStore := mock_store.NewMockPersistentStorageInterface(ctrl) + files := mock_store.NewMockFilePersistenceInterface(ctrl) + store.Store = mockStore + store.Files = files + + fd := &types.FileDef{ + ObjHeader: types.ObjHeader{Id: fid.String(), UpdatedAt: time.Date(2026, 8, 13, 9, 0, 0, 0, time.UTC)}, + TenantID: 7, + Status: types.UploadCompleted, + MimeType: "image/jpeg", + Location: "image.jpeg", + } + mockStore.EXPECT().GetMediaHandler().Return(handler) + files.EXPECT().Get(types.TenantID(7), fid.String()).Return(fd, nil) + files.EXPECT().UserHasAccess(types.TenantID(7), fid.String(), types.Uid(42)).Return(false, nil) + + if err := authorizeLargeFileRequestURL(7, 42, "/v0/file/s/file.jpeg"); err != types.ErrPermissionDenied { + t.Fatalf("expected permission denied, got %v", err) + } + if handler.headersCalled { + t.Fatal("media headers should not be called by authorization helper") + } +} + func TestContentDispositionAttachmentSanitizesName(t *testing.T) { fd := &types.FileDef{ ObjHeader: types.ObjHeader{Id: types.Uid(100).String()}, diff --git a/server/media/oss/oss.go b/server/media/oss/oss.go index 4e75174..8ee3fa0 100644 --- a/server/media/oss/oss.go +++ b/server/media/oss/oss.go @@ -258,7 +258,7 @@ func (oh *ossHandler) Headers(tenantID types.TenantID, method string, url *url.U "ETag": {`"` + fdef.ETag + `"`}, "Content-Type": {"application/json; charset=utf-8"}, "Cache-Control": {oh.conf.CacheControl}, - }, http.StatusPermanentRedirect, nil + }, http.StatusFound, nil } return nil, 0, nil } diff --git a/server/media/s3/s3.go b/server/media/s3/s3.go index 41e7539..bf65d37 100644 --- a/server/media/s3/s3.go +++ b/server/media/s3/s3.go @@ -248,16 +248,14 @@ func (ah *awshandler) Headers(tenantID types.TenantID, method string, url *url.U } if redirURL != "" { - // Return presigned URL with 308 Permanent redirect. Let the client cache the response. - // The original URL will stop working after a short period of time to prevent use of Tinode - // as a free file server. + // Return a temporary redirect to a short-lived presigned URL. return http.Header{ "Location": {redirURL}, "ETag": {`"` + fdef.ETag + `"`}, "Content-Type": {"application/json; charset=utf-8"}, "Cache-Control": {ah.conf.CacheControl}, }, - http.StatusPermanentRedirect, nil + http.StatusFound, nil } return nil, 0, nil }