diff --git a/services/thumbnails/pkg/preprocessor/dimensionguard_test.go b/services/thumbnails/pkg/preprocessor/dimensionguard_test.go new file mode 100644 index 0000000000..4eead95646 --- /dev/null +++ b/services/thumbnails/pkg/preprocessor/dimensionguard_test.go @@ -0,0 +1,105 @@ +//go:build !enable_vips + +package preprocessor + +import ( + "bytes" + "encoding/base64" + "encoding/json" + "image" + "image/jpeg" + + . "github.com/onsi/ginkgo" + . "github.com/onsi/gomega" + + thumbnailerErrors "github.com/opencloud-eu/opencloud/services/thumbnails/pkg/errors" +) + +// craftDimensionBomb encodes a tiny valid grayscale JPEG, then overwrites the +// SOF0 width/height so the header declares huge dimensions while the payload +// stays tiny. +func craftDimensionBomb(width, height uint16) []byte { + var buf bytes.Buffer + Expect(jpeg.Encode(&buf, image.NewGray(image.Rect(0, 0, 8, 8)), &jpeg.Options{Quality: 10})).To(Succeed()) + b := buf.Bytes() + for i := 0; i+9 < len(b); i++ { + if b[i] == 0xff && b[i+1] == 0xc0 { + b[i+5], b[i+6] = byte(height>>8), byte(height) + b[i+7], b[i+8] = byte(width>>8), byte(width) + return b + } + } + Fail("no SOF0 marker in encoded jpeg") + return nil +} + +var _ = Describe("ImageDecoder dimension guard", func() { + It("rejects a source whose declared dimensions exceed the limit before decoding", func() { + dec := ImageDecoder{limit: decodeLimit{maxWidth: 7680, maxHeight: 7680}} + _, err := dec.Convert(bytes.NewReader(craftDimensionBomb(20000, 20000))) + Expect(err).To(MatchError(thumbnailerErrors.ErrImageTooLarge)) + }) + + It("decodes an image within the limit", func() { + var buf bytes.Buffer + Expect(jpeg.Encode(&buf, image.NewGray(image.Rect(0, 0, 800, 600)), nil)).To(Succeed()) + dec := ImageDecoder{limit: decodeLimit{maxWidth: 7680, maxHeight: 7680}} + img, err := dec.Convert(bytes.NewReader(buf.Bytes())) + Expect(err).ToNot(HaveOccurred()) + Expect(img).ToNot(BeNil()) + }) + + It("applies no limit when the bounds are zero", func() { + dec := ImageDecoder{} + var buf bytes.Buffer + Expect(jpeg.Encode(&buf, image.NewGray(image.Rect(0, 0, 16, 16)), nil)).To(Succeed()) + _, err := dec.Convert(bytes.NewReader(buf.Bytes())) + Expect(err).ToNot(HaveOccurred()) + }) + + It("rejects an oversized gif before decoding", func() { + dec := GifDecoder{limit: decodeLimit{maxWidth: 7680, maxHeight: 7680}} + // LSD declares a 20000x20000 logical screen + g := []byte("GIF89a") + g = append(g, 0x20, 0x4e, 0x20, 0x4e, 0xf0, 0x00, 0x00) // 20000x20000, gct flag + g = append(g, bytes.Repeat([]byte{0}, 6)...) // minimal gct + terminator-ish + _, err := dec.Convert(bytes.NewReader(g)) + Expect(err).To(MatchError(thumbnailerErrors.ErrImageTooLarge)) + }) +}) + +var _ = Describe("dimension limit propagation", func() { + limit := decodeLimit{maxWidth: 7680, maxHeight: 7680} + opts := map[string]any{"maxInputWidth": 7680, "maxInputHeight": 7680} + + It("threads the limit into decoders that recurse into ForType", func() { + Expect(ForType("audio/mpeg", opts)).To(Equal(AudioDecoder{limit: limit})) + Expect(ForType("application/vnd.geogebra.pinboard", opts)).To(Equal(GgpDecoder{limit: limit})) + g, ok := ForType("application/vnd.geogebra.slides", opts).(GgsDecoder) + Expect(ok).To(BeTrue()) + Expect(g.limit).To(Equal(limit)) + }) + + It("rejects an oversized cover image embedded in a ggp file", func() { + bomb := craftDimensionBomb(20000, 20000) + payload := "data:image/jpeg;base64," + base64.StdEncoding.EncodeToString(bomb) + ggp := GGPStruct{} + ggp.Sections = append(ggp.Sections, struct { + Cards []struct { + Element struct { + Image struct{ Base64Image string } + } + } + }{Cards: []struct { + Element struct { + Image struct{ Base64Image string } + } + }{{Element: struct { + Image struct{ Base64Image string } + }{Image: struct{ Base64Image string }{Base64Image: payload}}}}}) + raw, err := json.Marshal(ggp) + Expect(err).ToNot(HaveOccurred()) + _, err = GgpDecoder{limit: limit}.Convert(bytes.NewReader(raw)) + Expect(err).To(MatchError(thumbnailerErrors.ErrImageTooLarge)) + }) +}) diff --git a/services/thumbnails/pkg/preprocessor/preprocessor.go b/services/thumbnails/pkg/preprocessor/preprocessor.go index f24b4f80db..7138c74eee 100644 --- a/services/thumbnails/pkg/preprocessor/preprocessor.go +++ b/services/thumbnails/pkg/preprocessor/preprocessor.go @@ -29,10 +29,14 @@ type FileConverter interface { } // GifDecoder is a converter for the gif file -type GifDecoder struct{} +type GifDecoder struct{ limit decodeLimit } // Convert reads the gif file and returns the thumbnail image -func (i GifDecoder) Convert(r io.Reader) (interface{}, error) { +func (i GifDecoder) Convert(r io.Reader) (any, error) { + r, err := i.limit.guardDimensions(r, gif.DecodeConfig) + if err != nil { + return nil, err + } img, err := gif.DecodeAll(r) if err != nil { return nil, errors.Wrap(err, `could not decode the image`) @@ -41,7 +45,10 @@ func (i GifDecoder) Convert(r io.Reader) (interface{}, error) { } // GgsDecoder is a converter for the geogebra slides file -type GgsDecoder struct{ thumbnailpath string } +type GgsDecoder struct { + thumbnailpath string + limit decodeLimit +} // Convert reads the ggs file and returns the thumbnail image func (g GgsDecoder) Convert(r io.Reader) (interface{}, error) { @@ -60,7 +67,7 @@ func (g GgsDecoder) Convert(r io.Reader) (interface{}, error) { if err != nil { return nil, err } - converter := ForType("image/png", nil) + converter := ForType("image/png", g.limit.opts()) if converter == nil { return nil, thumbnailerErrors.ErrNoConverterForExtractedImageFromGgsFile } @@ -75,7 +82,7 @@ func (g GgsDecoder) Convert(r io.Reader) (interface{}, error) { } // AudioDecoder is a converter for the audio file -type AudioDecoder struct{} +type AudioDecoder struct{ limit decodeLimit } // Convert reads the audio file and extracts the thumbnail image from the id3 tag func (i AudioDecoder) Convert(r io.Reader) (interface{}, error) { @@ -93,7 +100,7 @@ func (i AudioDecoder) Convert(r io.Reader) (interface{}, error) { return nil, thumbnailerErrors.ErrNoImageFromAudioFile } - converter := ForType(picture.MIMEType, nil) + converter := ForType(picture.MIMEType, i.limit.opts()) if converter == nil { return nil, thumbnailerErrors.ErrNoConverterForExtractedImageFromAudioFile } @@ -192,7 +199,7 @@ type GGPStruct struct { } // GgpDecoder is a converter for the geogebra pinboard file -type GgpDecoder struct{} +type GgpDecoder struct{ limit decodeLimit } // Convert reads the ggp file and returns the first thumbnail image func (j GgpDecoder) Convert(r io.Reader) (interface{}, error) { @@ -212,7 +219,14 @@ func (j GgpDecoder) Convert(r io.Reader) (interface{}, error) { return nil, err } - img, _, err := image.Decode(bytes.NewReader(b)) + r2, err := j.limit.guardDimensions(bytes.NewReader(b), func(rr io.Reader) (image.Config, error) { + cfg, _, err := image.DecodeConfig(rr) + return cfg, err + }) + if err != nil { + return nil, err + } + img, _, err := image.Decode(r2) return img, err } @@ -274,11 +288,61 @@ func drawWord(canvas *font.Drawer, word string, minX, maxX, incY, maxY fixed.Int } } +// decodeLimit bounds the source image dimensions a decoder accepts. A zero +// value on an axis disables the limit for that axis. +type decodeLimit struct { + maxWidth int + maxHeight int +} + +func decodeLimitFromOpts(opts map[string]any) decodeLimit { + l := decodeLimit{} + if v, ok := opts["maxInputWidth"].(int); ok { + l.maxWidth = v + } + if v, ok := opts["maxInputHeight"].(int); ok { + l.maxHeight = v + } + return l +} + +func (l decodeLimit) exceeded(width, height int) bool { + return (l.maxWidth > 0 && width > l.maxWidth) || (l.maxHeight > 0 && height > l.maxHeight) +} + +// opts renders the limit back into an options map so decoders that recurse +// into ForType can forward it to the nested image decoder. +func (l decodeLimit) opts() map[string]any { + return map[string]any{"maxInputWidth": l.maxWidth, "maxInputHeight": l.maxHeight} +} + +// guardDimensions reads only the image header (no pixel allocation) and +// rejects a source whose declared dimensions exceed the limit, before the +// full bitmap is decoded. It returns a reader that replays the consumed +// header so the caller can still decode from the start. A header that cannot +// be parsed is passed through unchecked, letting the real decoder report it. +func (l decodeLimit) guardDimensions(r io.Reader, config func(io.Reader) (image.Config, error)) (io.Reader, error) { + if l.maxWidth <= 0 && l.maxHeight <= 0 { + return r, nil + } + var head bytes.Buffer + cfg, err := config(io.TeeReader(r, &head)) + replay := io.MultiReader(&head, r) + if err != nil { + return replay, nil + } + if l.exceeded(cfg.Width, cfg.Height) { + return nil, thumbnailerErrors.ErrImageTooLarge + } + return replay, nil +} + // ForType returns the converter for the specified mimeType func ForType(mimeType string, opts map[string]interface{}) FileConverter { // We can ignore the error here because we parse it in IsMimeTypeSupported before and if it fails // return the service call. So we should only get here when the mimeType parses fine. mimeType, _, _ = mime.ParseMediaType(mimeType) + limit := decodeLimitFromOpts(opts) switch mimeType { case "text/plain": fontFileMap := "" @@ -310,18 +374,18 @@ func ForType(mimeType string, opts map[string]interface{}) FileConverter { fontLoader: fontLoader, } case "application/vnd.geogebra.slides": - return GgsDecoder{"_slide0/geogebra_thumbnail.png"} + return GgsDecoder{thumbnailpath: "_slide0/geogebra_thumbnail.png", limit: limit} case "application/vnd.geogebra.pinboard": - return GgpDecoder{} + return GgpDecoder{limit: limit} case "image/gif": - return GifDecoder{} + return GifDecoder{limit: limit} case "audio/flac": fallthrough case "audio/mpeg": fallthrough case "audio/ogg": - return AudioDecoder{} + return AudioDecoder{limit: limit} default: - return ImageDecoder{} + return ImageDecoder{limit: limit} } } diff --git a/services/thumbnails/pkg/preprocessor/preprocessor_imaging.go b/services/thumbnails/pkg/preprocessor/preprocessor_imaging.go index 4510e234e6..88394e63bd 100644 --- a/services/thumbnails/pkg/preprocessor/preprocessor_imaging.go +++ b/services/thumbnails/pkg/preprocessor/preprocessor_imaging.go @@ -3,6 +3,7 @@ package preprocessor import ( + "image" "io" "github.com/kovidgoyal/imaging" @@ -10,10 +11,20 @@ import ( ) // ImageDecoder is a converter for the image file -type ImageDecoder struct{} +type ImageDecoder struct{ limit decodeLimit } // Convert reads the image file and returns the thumbnail image -func (i ImageDecoder) Convert(r io.Reader) (interface{}, error) { +func (i ImageDecoder) Convert(r io.Reader) (any, error) { + // bound the declared dimensions before imaging.Decode allocates the full + // pixel buffer: a crafted header (e.g. 65535x65535) would otherwise OOM + // the worker, the downstream dimension guard only runs after the decode + r, err := i.limit.guardDimensions(r, func(rr io.Reader) (image.Config, error) { + cfg, _, err := image.DecodeConfig(rr) + return cfg, err + }) + if err != nil { + return nil, err + } img, err := imaging.Decode(r, imaging.AutoOrientation(true)) if err != nil { return nil, errors.Wrap(err, `could not decode the image`) diff --git a/services/thumbnails/pkg/preprocessor/preprocessor_test.go b/services/thumbnails/pkg/preprocessor/preprocessor_test.go index 531fe6a51e..17947f2540 100644 --- a/services/thumbnails/pkg/preprocessor/preprocessor_test.go +++ b/services/thumbnails/pkg/preprocessor/preprocessor_test.go @@ -81,14 +81,14 @@ var _ = Describe("ImageDecoder", func() { }) It("should decode a ggs", func() { - decoder := GgsDecoder{"_slide0/geogebra_thumbnail.png"} + decoder := GgsDecoder{thumbnailpath: "_slide0/geogebra_thumbnail.png"} img, err := decoder.Convert(fileReader) Expect(err).ToNot(HaveOccurred()) Expect(img).ToNot(BeNil()) }) It("should return an error if the ggs is invalid", func() { - decoder := GgsDecoder{"_slide0/geogebra_thumbnail.png"} + decoder := GgsDecoder{thumbnailpath: "_slide0/geogebra_thumbnail.png"} img, err := decoder.Convert(bytes.NewReader([]byte("not a ggs"))) Expect(err).To(HaveOccurred()) Expect(img).To(BeNil()) diff --git a/services/thumbnails/pkg/preprocessor/preprocessor_vips.go b/services/thumbnails/pkg/preprocessor/preprocessor_vips.go index 087bafb1b7..cdb015f1d0 100644 --- a/services/thumbnails/pkg/preprocessor/preprocessor_vips.go +++ b/services/thumbnails/pkg/preprocessor/preprocessor_vips.go @@ -6,15 +6,26 @@ import ( "io" "github.com/davidbyttow/govips/v2/vips" + + thumbnailerErrors "github.com/opencloud-eu/opencloud/services/thumbnails/pkg/errors" ) func init() { vips.LoggingSettings(nil, vips.LogLevelError) } -type ImageDecoder struct{} +type ImageDecoder struct{ limit decodeLimit } func (v ImageDecoder) Convert(r io.Reader) (interface{}, error) { + // NewImageFromReader is header-lazy, Width/Height read the header without + // materializing pixels: reject oversized sources before ThumbnailWithSize img, err := vips.NewImageFromReader(r) - return img, err + if err != nil { + return nil, err + } + if v.limit.exceeded(img.Width(), img.Height()) { + img.Close() + return nil, thumbnailerErrors.ErrImageTooLarge + } + return img, nil } diff --git a/services/thumbnails/pkg/service/grpc/v0/service.go b/services/thumbnails/pkg/service/grpc/v0/service.go index 92e93e6f9a..585e3e3035 100644 --- a/services/thumbnails/pkg/service/grpc/v0/service.go +++ b/services/thumbnails/pkg/service/grpc/v0/service.go @@ -53,6 +53,8 @@ func NewService(opts ...Option) decorators.DecoratedService { selector: options.GatewaySelector, preprocessorOpts: PreprocessorOpts{ TxtFontFileMap: options.Config.Thumbnail.FontMapFile, + MaxInputWidth: options.Config.Thumbnail.MaxInputWidth, + MaxInputHeight: options.Config.Thumbnail.MaxInputHeight, }, dataEndpoint: options.Config.Thumbnail.DataEndpoint, transferSecret: options.Config.Thumbnail.TransferSecret, @@ -77,6 +79,8 @@ type Thumbnail struct { // PreprocessorOpts holds the options for the preprocessor type PreprocessorOpts struct { TxtFontFileMap string + MaxInputWidth int + MaxInputHeight int } // GetThumbnail retrieves a thumbnail for an image @@ -164,11 +168,20 @@ func (g Thumbnail) handleCS3Source(ctx context.Context, req *thumbnailssvc.GetTh } defer r.Close() - ppOpts := map[string]interface{}{ - "fontFileMap": g.preprocessorOpts.TxtFontFileMap, + ppOpts := map[string]any{ + "fontFileMap": g.preprocessorOpts.TxtFontFileMap, + "maxInputWidth": g.preprocessorOpts.MaxInputWidth, + "maxInputHeight": g.preprocessorOpts.MaxInputHeight, } pp := preprocessor.ForType(sRes.GetInfo().GetMimeType(), ppOpts) img, err := pp.Convert(r) + if errors.Is(err, terrors.ErrImageTooLarge) { + return "", merrors.Forbidden(g.serviceID, "%s", err.Error()) + } + if err != nil { + g.logger.Error().Err(err).Msg("failed to convert image") + } + if img == nil || err != nil { return "", merrors.NotFound(g.serviceID, "could not get image") } @@ -256,11 +269,16 @@ func (g Thumbnail) handleWebdavSource(ctx context.Context, req *thumbnailssvc.Ge return "", merrors.InternalServerError(g.serviceID, "could not get image from source: %s", err.Error()) } defer r.Close() - ppOpts := map[string]interface{}{ - "fontFileMap": g.preprocessorOpts.TxtFontFileMap, + ppOpts := map[string]any{ + "fontFileMap": g.preprocessorOpts.TxtFontFileMap, + "maxInputWidth": g.preprocessorOpts.MaxInputWidth, + "maxInputHeight": g.preprocessorOpts.MaxInputHeight, } pp := preprocessor.ForType(sRes.GetInfo().GetMimeType(), ppOpts) img, err := pp.Convert(r) + if errors.Is(err, terrors.ErrImageTooLarge) { + return "", merrors.Forbidden(g.serviceID, "%s", err.Error()) + } if img == nil || err != nil { return "", merrors.NotFound(g.serviceID, "could not get image") }