diff --git a/pkg/modules/avatar/avatar.go b/pkg/modules/avatar/avatar.go index afd5b7b34..7c453542e 100644 --- a/pkg/modules/avatar/avatar.go +++ b/pkg/modules/avatar/avatar.go @@ -18,6 +18,7 @@ package avatar import ( "errors" + "image" "io" "strings" @@ -146,6 +147,19 @@ func StoreUploadedAvatar(s *xorm.Session, u *user.User, src io.ReadSeeker) error return err } + // The mimetype sniff above accepts image types we cannot actually store + // (e.g. SVG, WebP) because upload.StoreAvatarFile decodes via image.Decode, + // which only has the decoders registered process-wide by the imaging package + // (png, jpeg, gif, tiff, bmp). image.DecodeConfig uses those same decoders, so + // validating here rejects undecodable images with a 400 instead of failing + // deeper in storage with a 500. + if _, _, err := image.DecodeConfig(src); err != nil { + return ErrNotAnImage + } + if _, err := src.Seek(0, io.SeekStart); err != nil { + return err + } + u.AvatarProvider = "upload" if err := upload.StoreAvatarFile(s, u, src); err != nil { return err diff --git a/pkg/routes/api/v2/avatar_upload.go b/pkg/routes/api/v2/avatar_upload.go index 80893a53b..52b6038f6 100644 --- a/pkg/routes/api/v2/avatar_upload.go +++ b/pkg/routes/api/v2/avatar_upload.go @@ -31,9 +31,9 @@ import ( ) type avatarUploadInput struct { - // Broad allow-list because Huma's MimeTypeValidator rejects the part pre-handler; octet-stream covers programmatic clients. The real gate is the byte-level image check in avatar.StoreUploadedAvatar. + // Allow-list mirrors the image formats avatar.StoreUploadedAvatar can actually decode (the decoders registered process-wide by the imaging package: png, jpeg, gif, tiff, bmp); octet-stream covers programmatic clients. Huma's MimeTypeValidator rejects the part pre-handler, so anything advertised here must also pass the byte-level image check in avatar.StoreUploadedAvatar. Formats without a registered decoder (e.g. svg, webp) are intentionally excluded. RawBody huma.MultipartFormFiles[struct { - Avatar huma.FormFile `form:"avatar" contentType:"image/png,image/jpeg,image/gif,image/webp,image/svg+xml,application/octet-stream" required:"true" doc:"The avatar image to upload. Must be an image; it is resized server-side and re-encoded as PNG."` + Avatar huma.FormFile `form:"avatar" contentType:"image/png,image/jpeg,image/gif,image/tiff,image/bmp,application/octet-stream" required:"true" doc:"The avatar image to upload. Must be a decodable raster image (PNG, JPEG, GIF, TIFF or BMP); it is resized server-side and re-encoded as PNG."` }] } diff --git a/pkg/webtests/huma_avatar_upload_test.go b/pkg/webtests/huma_avatar_upload_test.go index 326d5a4ba..790039c5a 100644 --- a/pkg/webtests/huma_avatar_upload_test.go +++ b/pkg/webtests/huma_avatar_upload_test.go @@ -148,6 +148,29 @@ func TestAvatarUpload(t *testing.T) { assert.NotEqual(t, "upload", u.AvatarProvider) }) + t.Run("Undecodable image (SVG) rejected with 400", func(t *testing.T) { + // SVG sniffs as image/svg+xml, passing the mimetype prefix check, but + // image.Decode/DecodeConfig has no SVG decoder. Sent as octet-stream it + // bypasses the part content-type allow-list, so the byte-level decode + // validation in StoreUploadedAvatar is what must reject it. This must be + // a 400 (ErrNotAnImage), not a 500 from a failed decode deeper in storage. + e, err := setupTestEnv() + require.NoError(t, err) + token := humaTokenFor(t, &testuser1) + + svg := []byte(`` + + ``) + body, contentType := multipartAvatarBody(t, "avatar", "avatar.svg", svg) + rec := uploadAvatarRequest(t, e, body, contentType, token) + require.Equal(t, http.StatusBadRequest, rec.Code, "body: %s", rec.Body.String()) + + s := db.NewSession() + defer s.Close() + u, err := user.GetUserByID(s, testuser1.ID) + require.NoError(t, err) + assert.NotEqual(t, "upload", u.AvatarProvider) + }) + t.Run("Unauthenticated", func(t *testing.T) { e, err := setupTestEnv() require.NoError(t, err)