From 021c83367861f53a16ccaa2c42d3d13930f9c06e Mon Sep 17 00:00:00 2001
From: Dean Sheather <dean@deansheather.com>
Date: Sun, 17 Dec 2023 02:16:21 +1000
Subject: [PATCH] fix: copy thumbnail data to avoid buffer race
---
lib/thumbnailer/cache.go | 20 +++++++++++++-------
lib/thumbnailer/errors.go | 3 +++
lib/thumbnailer/transform.go | 27 ++++++++++++++++++---------
objects.sql | 32 +++++++++++++++++---------------
4 files changed, 51 insertions(+), 31 deletions(-)
diff --git a/lib/thumbnailer/cache.go b/lib/thumbnailer/cache.go
index 38f5130..412ff9c 100644
--- a/lib/thumbnailer/cache.go
+++ b/lib/thumbnailer/cache.go
@@ -25,6 +25,9 @@ func NewThumbnailCache(directory, thumbnailerURL string) *ThumbnailCache {
// GetThumbnail returns a thumbnail that is cached. If no cached copy exists, a
// exists, a NoCachedCopy error is returned.
func (c *ThumbnailCache) GetThumbnail(key string) (io.ReadCloser, error) {
+ if key == "" {
+ return nil, NoKeySpecified
+ }
path := filepath.Join(c.Directory, key)
data, err := os.Open(path)
if os.IsNotExist(err) {
@@ -35,27 +38,30 @@ func (c *ThumbnailCache) GetThumbnail(key string) (io.ReadCloser, error) {
// SetThumbnail stores a thumbnail with the specified key.
func (c *ThumbnailCache) SetThumbnail(key string, data io.Reader) error {
+ if key == "" {
+ return NoKeySpecified
+ }
path := filepath.Join(c.Directory, key)
file, err := os.Create(path)
- defer file.Close()
if err != nil {
return err
}
+ defer file.Close()
_, err = io.Copy(file, data)
+ if err != nil {
+ _ = os.Remove(path)
+ }
return err
}
// Transform generates a thumbnail and caches it.
func (c *ThumbnailCache) Transform(key string, contentType string, data io.Reader) error {
+ if key == "" {
+ return NoKeySpecified
+ }
outputImage, err := Transform(c.ThumbnailerURL, contentType, data)
if err != nil {
return err
}
return c.SetThumbnail(key, outputImage)
}
-
-// DeleteThumbnail deletes a thumbnail from the cache.
-func (c *ThumbnailCache) DeleteThumbnail(key string) error {
- path := filepath.Join(c.Directory, key)
- return os.Remove(path)
-}
diff --git a/lib/thumbnailer/errors.go b/lib/thumbnailer/errors.go
index 2c655c6..6a2f1be 100644
--- a/lib/thumbnailer/errors.go
+++ b/lib/thumbnailer/errors.go
@@ -14,3 +14,6 @@ var NoCachedCopy error = &thumbnailerError{"no cached copy of the thumbnail requ
// InputTooLarge means that the pixel size of the input image is too big to be thumbnailed.
var InputTooLarge error = &thumbnailerError{"the input size in pixels is too large"}
+
+// NoKeySpecified means that no key was specified.
+var NoKeySpecified error = &thumbnailerError{"no key specified"}
diff --git a/lib/thumbnailer/transform.go b/lib/thumbnailer/transform.go
index a5b4456..ff91cf8 100644
--- a/lib/thumbnailer/transform.go
+++ b/lib/thumbnailer/transform.go
@@ -11,10 +11,10 @@ import (
// Accepted MIME types for thumbnails in map for easy checking
var thumbnailMIMETypes = map[string]struct{}{
- "image/gif": struct{}{},
- "image/jpeg": struct{}{},
- "image/png": struct{}{},
- "image/webp": struct{}{},
+ "image/gif": {},
+ "image/jpeg": {},
+ "image/png": {},
+ "image/webp": {},
}
// AcceptedMIMEType checks if a MIME type is suitable for thumbnailing.
@@ -27,15 +27,19 @@ func AcceptedMIMEType(mime string) bool {
// Transform takes an image io.Reader and sends it to the thumbnailer service
// to be transcoded into a thumbnail.
func Transform(thumbnailerURL, contentType string, data io.Reader) (*bytes.Buffer, error) {
- // Set request and response
+ if !AcceptedMIMEType(contentType) {
+ return nil, errors.Errorf("invalid MIME type: %s", contentType)
+ }
+
req := fasthttp.AcquireRequest()
res := fasthttp.AcquireResponse()
defer func() {
fasthttp.ReleaseRequest(req)
fasthttp.ReleaseResponse(res)
}()
-
req.Reset()
+ res.Reset()
+
req.Header.SetMethod("POST")
req.SetRequestURI(thumbnailerURL)
req.Header.Set("Content-Type", contentType)
@@ -43,9 +47,7 @@ func Transform(thumbnailerURL, contentType string, data io.Reader) (*bytes.Buffe
if err != nil {
return nil, errors.Wrap(err, "failed to copy data to request")
}
- res.Reset()
- // Do request
err = fasthttp.Do(req, res)
if err != nil {
return nil, errors.Wrap(err, "failed to make request to thumbnailer service")
@@ -54,5 +56,12 @@ func Transform(thumbnailerURL, contentType string, data io.Reader) (*bytes.Buffe
return nil, errors.Errorf("thumbnailer service failed to create thumbnail: %s", string(res.Body()))
}
- return bytes.NewBuffer(res.Body()), nil
+ // Copy the response body to a buffer so we can return it safely.
+ // ReleaseResponse will return the buffer to the pool.
+ buf := bytes.NewBuffer(nil)
+ err = res.BodyWriteTo(buf)
+ if err != nil {
+ return nil, errors.Wrap(err, "failed to copy response body to buffer")
+ }
+ return buf, nil
}
diff --git a/objects.sql b/objects.sql
index 62a6fd3..6ed6333 100644
--- a/objects.sql
+++ b/objects.sql
@@ -1,19 +1,21 @@
-- "objects" table schema
-CREATE TABLE IF NOT EXISTS objects (
- bucket_key VARCHAR(1088) NOT NULL UNIQUE, -- bucket + key (unique)
- bucket VARCHAR(20) NOT NULL, -- uint64 bucket ID ("public" for public bucket)
- "key" VARCHAR(1024) NOT NULL, -- Full bucket path to file (including directory)
- random_key VARCHAR(1024), -- random key if used
- dir VARCHAR(1024) NOT NULL, -- Directory of file (with trailing slash)
- "type" integer NOT NULL DEFAULT 0, -- Object type enumerable (0 = file, 1 = redirect)
- dest_url VARCHAR(1024) DEFAULT NULL, -- Destination URL for redirect object (only when object.type == 1)
- content_type VARCHAR(255) DEFAULT 'application/octet-stream', -- Content-Type of file
- content_length INT DEFAULT NULL, -- Content-Length of file
- associated_user VARCHAR(36) DEFAULT NULL, -- ID of user who uploaded file
- created_at TIMESTAMP NOT NULL DEFAULT CURRENT_TIMESTAMP, -- File creation timestamp
- deleted_at TIMESTAMP DEFAULT NULL, -- Deletion timestamp
- delete_reason VARCHAR(256) DEFAULT NULL, -- Deletion reason
- md5_hash VARCHAR(32) DEFAULT NULL -- MD5 hash of file contents (or destination URL)
+CREATE TABLE objects (
+ bucket_key character varying(1088) NOT NULL, -- ${bucket}/${key} (unique)
+ bucket character varying(20) NOT NULL, -- Bucket ID ("public" for public bucket)
+ key character varying(1024) NOT NULL, -- Full bucket path to file (including directory)
+ dir character varying(1024) NOT NULL, -- Directory of file (with trailing slash)
+ type integer DEFAULT 0 NOT NULL, -- Object type enumerable (0 = file, 1 = redirect)
+ backend_file_id character varying(33) DEFAULT NULL, -- ID of file in backend storage
+ dest_url character varying(4096) DEFAULT NULL, -- Destination URL for redirect object (only when object.type == 1)
+ content_type character varying(255) DEFAULT 'application/octet-stream', -- Content-Type of file
+ content_length integer, -- Content-Length of file
+ created_at timestamp without time zone DEFAULT now() NOT NULL, -- File creation timestamp
+ random_key character varying(1024) DEFAULT NULL, -- Random key if used
+ associated_user character varying(36) DEFAULT NULL, -- ID of user who uploaded file
+ deleted_at timestamp without time zone, -- Deletion timestamp
+ delete_reason character varying(256) DEFAULT NULL::character varying, -- Deletion reason
+ sha256_hash bytea, -- SHA256 hash of file contents (or destination URL)
+ md5_hash bytea -- MD5 hash of file contents (or destination URL)
);
-- Test file object: /index.md
--
GitLab