diff --git a/docs/database/attachments.md b/docs/database/attachments.md index 95b5133..6478949 100644 --- a/docs/database/attachments.md +++ b/docs/database/attachments.md @@ -93,7 +93,9 @@ Originals in JPEG, PNG, GIF and WebP can be thumbnailed. The thumbnailer cannot `attach.Store` turns an upload into a `system_files` row. It takes an `attach.Upload` (the client's file name, the body and the public flag) and an `attach.Limits`, writes the bytes under the WinterCMS partition key and inserts an unattached row whose `sort_order` equals its id, as WinterCMS's Sortable trait does. The caller attaches the row later by setting `Field`, `AttachmentType` and `AttachmentID`, usually when the record's form is saved. When the row insert fails, the blob is deleted again. -The client file name never reaches a blob key. The disk name is 22 random hexadecimal characters plus the lower-cased extension, and the extension must be one to ten letters or digits from the allowed list: `Limits.Extensions`, or, when that is empty, `attach.DefaultImageExtensions` in image mode (jpg, jpeg, png, gif and webp) and `attach.DefaultFileExtensions` otherwise. The file list is WinterCMS's default list without the types that can carry script: svg, js, map, css, less, scss, swf and xml. A refused extension returns `attach.ErrFileType`. +The client file name never reaches a blob key. The disk name is 22 random hexadecimal characters plus an extension, and the client's lower-cased extension must be one to ten letters or digits from the allowed list: `Limits.Extensions`, or, when that is empty, `attach.DefaultImageExtensions` in image mode (jpg, jpeg, png, gif and webp) and `attach.DefaultFileExtensions` otherwise. The file list is WinterCMS's default list without the types that can carry script: svg, js, map, css, less, scss, swf and xml. A refused extension returns `attach.ErrFileType`. + +The disk name's extension follows the content, not the client's name, because a web server or CDN in front of the bucket picks the Content-Type from the extension. `attach.DiskExtension` derives it from the sniffed type and the client's extension. For a specific type such as PNG, JPEG, PDF, MP3 or WebM, the client's extension is kept when it names that type, else the type's usual extension is used: a PNG uploaded as `x.html` (with `html` allowed) is stored as `.png`, and its URL ends in `.png`. For a generic sniff (plain text, HTML, XML, a zip container or unknown bytes) the client's extension is kept, so `report.docx` stays `.docx` and an HTML body uploaded as `note.txt` stays `.txt`, unless the extension is one a browser renders or runs (html, svg, xml, js, css, php and the like), which becomes `.txt` for text and `.bin` otherwise. `file_name` always keeps the client's name, so downloads still carry it. The content type is sniffed from the first bytes of the body, never taken from the client. `Limits.MIMETypes` narrows it further: an entry with a slash is a MIME pattern such as `image/png` or `image/*`, an entry without one is an extension, and a file that matches no entry returns `attach.ErrMIMEType`. `Limits.MaxBytes` is enforced while the body streams into the bucket; a body one byte over the limit returns `attach.ErrTooLarge` and leaves no blob and no row behind. diff --git a/docs/services/storage.md b/docs/services/storage.md index 8c8866b..6c40a2b 100644 --- a/docs/services/storage.md +++ b/docs/services/storage.md @@ -103,6 +103,8 @@ The route looks up the bucket and the database on each request: `route:list` lis Without the key, the application decides where files are served: mount `attach.StaticHandlerPublic` itself, or put a web server or CDN in front of the bucket directory. Either way, serve uploads from a separate origin when you can; the [Attachments](../database/attachments.md) page explains why. The built-in route is meant for development and for deployments that cannot split the origin yet. +> **Deploy note.** When a static server such as nginx, or a CDN, serves the uploads directory or bucket directly, it picks the Content-Type from the file extension and sends none of the route's headers. `attach.Store` names each file after its sniffed content (`attach.DiskExtension`), so a PNG uploaded as `x.html` is stored as `.png`, but configure the server as the route behaves anyway: send `X-Content-Type-Options: nosniff` on every upload, and serve only the extensions the application accepts (for example the image and document types in `attach.DefaultFileExtensions`), answering 404 for anything else. + ## Upload size Every non-raw route has a request body limit of `http.body_limits.default_bytes`. A route that accepts uploads raises its own limit with the `body.limit:` middleware; see [Routing](routing.md). `http.body_limits.upload_bytes` is required and validated at start-up, but the framework applies it to no route; a plugin that wants its upload routes to follow it reads it in `Register` and puts the value in the route's `body.limit`. diff --git a/modules/lagoon/README.md b/modules/lagoon/README.md index b78c215..7f1e0c4 100644 --- a/modules/lagoon/README.md +++ b/modules/lagoon/README.md @@ -38,7 +38,7 @@ Use cases: - Lifecycle and relations: hook interfaces matching GORM's native method names (`lagoon.HasBeforeCreate`, `lagoon.HasBeforeSave`, `lagoon.HasBeforeDelete`, `lagoon.HasAfterDelete`) plus `lagoon.HasBeforeValidate`; `lagoon.WithSoftDeleteCascade` runs a cascade inside the parent delete; `lagoon.RegisterJoinTable` wires pivot models with business columns. - Deferred binding: WinterCMS's `deferred_bindings` table holds the uploads and related-record changes of a form whose record is not saved yet. Every operation takes a `lagoon.DeferredKey` (the form's session key, the owning backend admin's id and the master record's morph type from `lagoon.MorphType`) and never reads or changes another admin's rows, since each row stores `backend_user_id`. `lagoon.DeferredBind` and `lagoon.DeferredUnbind` port WinterCMS's duplicate and cancel rules: a repeated bind writes nothing, and an unbind of a slave with a pending bind deletes that bind and returns it so the caller can remove what it created. `lagoon.DeferredBindings` reads and locks a session's bindings for the save that commits them, `lagoon.DeferredForget` deletes them once applied, and `lagoon.DeferredSlaves` is the subquery a list uses to include pending rows. A child created under deferral carries the `lagoon.DeferredEnvelope` (`{"created":true,"pivot":{...},"upload_id":"..."}`) in `pivot_data`. `lagoon.PurgeDeferred` removes expired bindings: it deletes an unattached `system_files` row a bind points at, and its blobs only after the commit, deletes a child only when its binding carries the created envelope, keeps records that were only linked, and locks each batch with `FOR UPDATE SKIP LOCKED`. - Imports from Laravel: `lagoon.DecryptLaravelPayload` decrypts Laravel `encrypted` payloads with the old application key, for one-off data imports. -- Attachments (`attach`): the `attach.File` model for `system_files` rows, WinterCMS-compatible partitioned storage keys (`attach.BlobKey`, `attach.PartitionDirectory`), an S3 bucket option behind the `attach.S3BuildTag` build tag (an untagged binary refuses an `s3://` bucket URL at boot with an error naming the tag), public URLs (`attach.PublicURL` for any key, `attach.File.URL` for an original, matching WinterCMS's `File::getPath()` under the WinterCMS layout), on-demand thumbnails through `attach.File.Thumb` for JPEG, PNG, GIF and WebP originals (a WebP original's thumbnail is JPEG bytes under its `.webp` name, since WebP cannot be encoded; a missing, undecodable or oversized original gets WinterCMS's broken-image picture, `attach.BrokenImagePNG`, as its thumbnail, as `File::makeThumb` does), storing uploads through `attach.Store` (a server-generated disk name, an extension allow-list with `attach.DefaultImageExtensions` and `attach.DefaultFileExtensions` as defaults, a MIME filter, a size limit enforced while streaming and, in image mode, the `attach.IsAllowedImage` content guard), attachment relation declarations (`attach.Relation`, `attach.HasRelations`), static serving with an optional `is_public` gate (`attach.StaticHandlerPublic`), and a two-phase delete that removes blobs only after the database transaction commits (`attach.DeleteForOwner`, `attach.DeleteKeys`). +- Attachments (`attach`): the `attach.File` model for `system_files` rows, WinterCMS-compatible partitioned storage keys (`attach.BlobKey`, `attach.PartitionDirectory`), an S3 bucket option behind the `attach.S3BuildTag` build tag (an untagged binary refuses an `s3://` bucket URL at boot with an error naming the tag), public URLs (`attach.PublicURL` for any key, `attach.File.URL` for an original, matching WinterCMS's `File::getPath()` under the WinterCMS layout), on-demand thumbnails through `attach.File.Thumb` for JPEG, PNG, GIF and WebP originals (a WebP original's thumbnail is JPEG bytes under its `.webp` name, since WebP cannot be encoded; a missing, undecodable or oversized original gets WinterCMS's broken-image picture, `attach.BrokenImagePNG`, as its thumbnail, as `File::makeThumb` does), storing uploads through `attach.Store` (a server-generated disk name whose extension follows the sniffed content through `attach.DiskExtension`, so a PNG named `x.html` is stored as `.png`, an extension allow-list with `attach.DefaultImageExtensions` and `attach.DefaultFileExtensions` as defaults, a MIME filter, a size limit enforced while streaming and, in image mode, the `attach.IsAllowedImage` content guard), attachment relation declarations (`attach.Relation`, `attach.HasRelations`), static serving with an optional `is_public` gate (`attach.StaticHandlerPublic`), and a two-phase delete that removes blobs only after the database transaction commits (`attach.DeleteForOwner`, `attach.DeleteKeys`). ## Usage @@ -202,7 +202,8 @@ func (p *Plugin) Migrations() []*gormigrate.Migration { | `attach.DeleteForOwner` | Deletes an owner's attachment rows in a transaction and reports their blob keys. | | `attach.DeleteKeys` | Deletes blobs, including thumbnails, after the transaction commits. | | `attach.Migrations` | Creates the `system_files` table. | -| `attach.Store` | Stores an upload as an unattached `system_files` row with `sort_order` equal to its id, after the type, size and image checks. | +| `attach.Store` | Stores an upload as an unattached `system_files` row with `sort_order` equal to its id, after the type, size and image checks; the disk name's extension comes from `attach.DiskExtension`, `file_name` keeps the client's name. | +| `attach.DiskExtension` | The stored extension for a sniffed content type and the client's extension: the client's when it matches a specific type, else that type's canonical one; under a generic sniff the client's unless it is script-capable (then `txt` or `bin`). | | `attach.Upload` | The client file name, body and public flag of one upload. | | `attach.Limits` | Size limit, allowed extensions, allowed MIME types and image mode for `attach.Store`. | | `attach.ErrTooLarge` | Returned by `attach.Store` for a body over `MaxBytes`. | diff --git a/modules/lagoon/attach/diskext.go b/modules/lagoon/attach/diskext.go new file mode 100644 index 0000000..3873e8c --- /dev/null +++ b/modules/lagoon/attach/diskext.go @@ -0,0 +1,78 @@ +package attach + +import ( + "slices" + "strings" +) + +// sniffedExtensions maps a specific sniffed content type to the extensions +// that may name it on disk; the first entry is the canonical one. The table +// is fixed in code on purpose: package mime's answers depend on the host's +// mime tables. +var sniffedExtensions = map[string][]string{ + "image/png": {"png"}, + "image/jpeg": {"jpg", "jpeg", "jpe"}, + "image/gif": {"gif"}, + "image/webp": {"webp"}, + "image/bmp": {"bmp"}, + "image/x-icon": {"ico"}, + "image/vnd.microsoft.icon": {"ico"}, + "application/pdf": {"pdf"}, + "audio/mpeg": {"mp3"}, + "audio/wave": {"wav"}, + "audio/wav": {"wav"}, + "application/ogg": {"ogg", "oga", "opus"}, + "audio/ogg": {"ogg", "oga", "opus"}, + "video/webm": {"webm", "mkv"}, + "video/mp4": {"mp4", "m4v"}, + "font/woff2": {"woff2"}, + "font/woff": {"woff"}, + "font/ttf": {"ttf"}, + "font/otf": {"otf"}, + "application/vnd.ms-fontobject": {"eot"}, + "application/x-rar-compressed": {"rar"}, + "application/x-gzip": {"gz", "tgz"}, +} + +// scriptCapableExtensions are extensions a browser or web server may run or +// render as a document. They are never kept on disk under a generic sniff. +var scriptCapableExtensions = []string{ + "html", "htm", "xhtml", "xht", "shtml", "svg", "svgz", "xml", "xsl", "xslt", + "js", "mjs", "css", "swf", "php", "phtml", "pht", "phar", +} + +// DiskExtension returns the extension, without the dot, under which an +// upload is stored, so a web server that picks the Content-Type from the +// extension serves what the bytes are rather than what the client named +// them. contentType is the sniffed base media type (http.DetectContentType +// without parameters) and clientExt the client's allow-listed extension. +// +// For a specific sniffed type (PNG, JPEG, GIF, WebP, PDF, MP3, WebM, fonts +// and so on) the client extension is kept when it belongs to that type, +// else the type's canonical extension is used: a PNG named x.html is stored +// as .png. Any other type is generic (application/octet-stream, +// application/zip, text/plain, text/html, text/xml): the client extension +// is kept, so .docx, .xlsx and .ics survive, unless it is script-capable +// (html, svg, xml, js, css, php and the like), which becomes txt for a +// text/* type and bin otherwise. +func DiskExtension(contentType, clientExt string) string { + ct := strings.ToLower(strings.TrimSpace(contentType)) + if base, _, ok := strings.Cut(ct, ";"); ok { + ct = strings.TrimSpace(base) + } + ext := strings.ToLower(strings.TrimPrefix(strings.TrimSpace(clientExt), ".")) + + if exts, ok := sniffedExtensions[ct]; ok { + if slices.Contains(exts, ext) { + return ext + } + return exts[0] + } + if extPattern.MatchString(ext) && !slices.Contains(scriptCapableExtensions, ext) { + return ext + } + if strings.HasPrefix(ct, "text/") { + return "txt" + } + return "bin" +} diff --git a/modules/lagoon/attach/store.go b/modules/lagoon/attach/store.go index 52ae1cf..78d8dc3 100644 --- a/modules/lagoon/attach/store.go +++ b/modules/lagoon/attach/store.go @@ -95,7 +95,9 @@ type Limits struct { // checked against the sniffed type, or the extension's registered type when // the sniff only says application/octet-stream (else ErrMIMEType). The body // is then streamed into bucket at BlobKey of a server-generated disk name (22 -// random lowercase hex characters, a dot and the extension); a body longer +// random lowercase hex characters, a dot and the extension DiskExtension +// derives from the sniffed type and the client extension, so a PNG named +// x.html is stored as .png; file_name keeps the client's name); a body longer // than Limits.MaxBytes aborts the write, deletes the key and returns // ErrTooLarge. Finally it inserts the row with empty attachment columns, // is_public from Upload.Public, the byte size and the content type, and sets @@ -138,7 +140,8 @@ func Store(ctx context.Context, db *gorm.DB, bucket *blob.Bucket, in Upload, lim if lim.Image && !IsAllowedImage(head) { return nil, ErrNotImage } - contentType := baseMediaType(http.DetectContentType(head)) + sniffed := baseMediaType(http.DetectContentType(head)) + contentType := sniffed if contentType == "application/octet-stream" { if byExt := baseMediaType(mime.TypeByExtension("." + ext)); byExt != "" { contentType = byExt @@ -148,7 +151,7 @@ func Store(ctx context.Context, db *gorm.DB, bucket *blob.Bucket, in Upload, lim return nil, fmt.Errorf("%w: %s", ErrMIMEType, contentType) } - diskName, err := newDiskName(ext) + diskName, err := newDiskName(DiskExtension(sniffed, ext)) if err != nil { return nil, err } diff --git a/modules/lagoon/attach/store_test.go b/modules/lagoon/attach/store_test.go index 6e3e6fa..a907e73 100644 --- a/modules/lagoon/attach/store_test.go +++ b/modules/lagoon/attach/store_test.go @@ -1,6 +1,7 @@ package attach_test import ( + "archive/zip" "bytes" "context" "errors" @@ -284,3 +285,61 @@ func TestStore(t *testing.T) { } }) } + +// TestStoreDiskExtensionSmoke is the stored-XSS regression (D-15): the disk +// name's extension follows the sniffed content, file_name keeps the client's +// name, and documents whose sniff is generic keep their extension. +func TestStoreDiskExtensionSmoke(t *testing.T) { + if testing.Short() { + t.Skip("requires testcontainers postgres") + } + ctx := t.Context() + gdb := attachGorm(t) + if err := lagoon.Migrate(gdb, nil); err != nil { + t.Fatal(err) + } + bucket := memblob.OpenBucket(nil) + t.Cleanup(func() { _ = bucket.Close() }) + lim := attach.Limits{Extensions: []string{"html", "png", "docx", "txt"}} + + // An 8-byte PNG signature plus an IHDR chunk sniffs as image/png. + pngBody := append([]byte("\x89PNG\r\n\x1a\n"), []byte("\x00\x00\x00\x0dIHDR\x00\x00\x00\x01\x00\x00\x00\x01\x08\x02\x00\x00\x00\x90wS\xde")...) + f, err := attach.Store(ctx, gdb, bucket, attach.Upload{FileName: "x.html", Body: bytes.NewReader(pngBody), Public: true}, lim) + if err != nil { + t.Fatal(err) + } + if !strings.HasSuffix(f.DiskName, ".png") || !strings.HasSuffix(f.URL(), ".png") { + t.Fatalf("PNG named x.html: disk name %q, URL %q, want .png", f.DiskName, f.URL()) + } + if f.FileName != "x.html" || f.ContentType != "image/png" { + t.Fatalf("PNG named x.html: file_name %q, content_type %q", f.FileName, f.ContentType) + } + + var zipBody bytes.Buffer + zw := zip.NewWriter(&zipBody) + w, err := zw.Create("word/document.xml") + if err != nil { + t.Fatal(err) + } + if _, err := w.Write([]byte("")); err != nil { + t.Fatal(err) + } + if err := zw.Close(); err != nil { + t.Fatal(err) + } + doc, err := attach.Store(ctx, gdb, bucket, attach.Upload{FileName: "report.docx", Body: bytes.NewReader(zipBody.Bytes())}, lim) + if err != nil { + t.Fatal(err) + } + if !strings.HasSuffix(doc.DiskName, ".docx") || doc.FileName != "report.docx" { + t.Fatalf("docx: disk name %q, file_name %q", doc.DiskName, doc.FileName) + } + + note, err := attach.Store(ctx, gdb, bucket, attach.Upload{FileName: "note.txt", Body: strings.NewReader("")}, lim) + if err != nil { + t.Fatal(err) + } + if !strings.HasSuffix(note.DiskName, ".txt") || note.FileName != "note.txt" { + t.Fatalf("HTML body named note.txt: disk name %q, file_name %q", note.DiskName, note.FileName) + } +}