fix(19.3-03): name stored uploads after their sniffed content through attach.DiskExtension

- DiskExtension: a fixed table of specific sniffed types keeps a matching client
  extension or uses the canonical one; generic sniffs keep the client extension
  unless it is script-capable (then txt or bin); never asks package mime
- Store names the blob with DiskExtension(sniffed, ext); file_name, the allow-list
  and the stored content_type are unchanged
- TestStoreDiskExtensionSmoke: a PNG uploaded as x.html is stored and served as .png,
  report.docx keeps .docx, an HTML body named note.txt keeps .txt
- attachments docs on disk names, storage deploy note on nosniff and allow-lists,
  lagoon README
This commit is contained in:
Jakub Zych
2026-10-15 18:09:49 +02:00
parent 2382040b25
commit 2e22e9f9a6
6 changed files with 151 additions and 6 deletions

View File

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

View File

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

View File

@@ -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("<w:document/>")); 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("<html><body><script>alert(1)</script></body></html>")}, 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)
}
}