fix(api/v2): reject non-decodable images (e.g. SVG) on avatar upload with 400

This commit is contained in:
kolaente 2026-06-02 09:24:30 +02:00 committed by kolaente
parent d2319e1257
commit b18e051ab3
3 changed files with 39 additions and 2 deletions

View File

@ -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

View File

@ -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."`
}]
}

View File

@ -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(`<?xml version="1.0" encoding="UTF-8"?>` +
`<svg xmlns="http://www.w3.org/2000/svg" width="8" height="8"><rect width="8" height="8" fill="#abc"/></svg>`)
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)