mirror of
https://github.com/PerpetualSoftware/pad.git
synced 2026-09-10 15:05:40 +00:00
fix(attachments): make allowlisted formats reachable by recognising their magic (BUG-2963 PR A) (#1304)
* fix(attachments): types the allowlist names are reachable from their own bytes (BUG-2963 PR A) BUG-2961 fixed one member of a class. The class, measured over 44 real files covering 41 extensions: 25 of 48 upload-allowlist entries could never be the type an upload was stored under, so the door refused — or silently retyped — files every surface advertises as supported. This is the sniff-side half. Nothing here trusts a filename to introduce a type; the trust decisions (the text family, the audio/video category split, the CFB office trio) are a separate change. - F1: two spelling aliases, the shape audio/wave and application/x-gzip already have. video/avi is a pure spelling difference. application/ogg is not: Ogg is a container and the allowlist has no video/ogg, so aliasing types an Ogg video as audio. Ruled the better of two answers, since the alternative is refusing every Ogg, and the reason is on the alias line. - F2: magic-byte pre-checks for tar (ustar at offset 257, which is why no prefix matcher finds it), bzip2, 7z and FLAC. Consulted ONLY where the stdlib returned application/octet-stream, so they can add a detection and never replace one. Raw AAC is resolved in ValidateUpload instead: twelve bits of sync is too weak to act on alone, so it is gated on the .aac extension as well — the bytes must still carry the sync, the extension only decides whether a weak signature may speak. - F3: the one that was accepted rather than refused, and so survived the first pass. The mimesniff table maps the bare EBML magic to video/webm with no DocType check, so a Matroska file uploaded fine and was stored as WebM. A DocType read separates them; an EBML header carrying neither string keeps the stdlib verdict, so the fallback is the behaviour that shipped before. - F6: application/javascript, text/yaml and application/xml leave the allowlist — no extension reaches those spellings and no sniff emits them. That deletion has a trap: extMIMEMap's values are looked up in `allowed`, and an extension naming a removed type is the mechanism that refuses .svg and .exe, so .xml now names text/xml. A test covers the .xml upload, not just the three lookups. audio/webm is equally unreachable and STAYS by ruling, with the comment a future tidy-up will read. Fixtures are real encoder output truncated to the 512 bytes the door reads, with provenance and one recorded gap in the testdata README. The measurement harness ships in neither PR. Refs: BUG-2963, BUG-2961 * docs(attachments): bring prose that F6 falsified back to true (BUG-2963) Three client comments and one store filter described the server's category for `application/javascript`, which stopped having one when F6 removed it from the allowlist. Nothing in the previous commit's diff points at these lines, which is the whole reason the sweep exists (team CONVE-23). Behaviour is unchanged in all four. The web allowlists still exclude `application/javascript` deliberately — the string can reach the client from somewhere that is not our upload door — and the store's category filter still matches it, because it buckets rows that EXIST rather than deciding what may be created: a filter that stops matching a type costs a row nobody can find, while one that matches a type no row carries costs nothing. Refs: BUG-2963 * fix(attachments): admit a format by its own integrity check, not by its magic (BUG-2963) Round 1 of adversarial review walked straight through the first version of these signatures. A prefix match is not a format, and on a default-deny door the difference is the whole point: - a real, EXECUTING ELF binary with "ustar" in unused padding at offset 257 was stored as application/x-tar - the six-byte 7z signature, alone, was a 7z archive - "fLaC\x00" was a FLAC stream; "BZh9\x00" was a bzip2 stream - FF F1 00 — three bytes — was an AAC frame - a WebM carrying "matroska" inside a legal Void element was stored as Matroska, and a Matroska with 40 bytes of Void padding was stored as WebM, leaving the mistyping this change exists to fix live for any padded file - a real VP8-in-Ogg video was accepted as audio/ogg, category audio, INLINE Every one was refused before the change and accepted after it. So each format is now admitted only by the integrity check the format itself defines: tar's header checksum, bzip2's block magic, 7z's start-header CRC, FLAC's mandatory 34-byte STREAMINFO, ADTS's reserved sampling-frequency index and layer bits. The EBML DocType is PARSED — a real element walk that skips Void — rather than searched for as a string in a fixed window. The Ogg alias is gated on the first packet naming an audio codec (Vorbis, Opus, FLAC, Speex); Theora and VP8 in Ogg stay refused exactly as before, and video/ogg is deliberately not added, because admitting a format is a review and not a side effect. Three comments asserted things that were false and are corrected rather than softened: that these formats always sniff as octet-stream (a tar whose first member is BM.txt sniffs image/bmp), that a .aac without a sync is refused (it is not, if its bytes are another audio type), and that deleting one switch case would fail one named test (this project's own mutation matrix already showed otherwise, and the comment was written against that data). Tests gained the cases that decide it: every near-miss body above as an exact upload, the four EBML directions including both adversarial files, the ADTS guards isolated so exactly one mutation kills each, and length boundaries at the offsets each check indexes past — the previous negatives padded everything to 512 bytes, which hid every length guard. Refs: BUG-2963 * test(attachments): kill two mutants the round-1 tests left alive (BUG-2963) Both survived the expanded matrix, and both for the same reason round 1 kept finding: a negative case that fails for a reason other than the guard it names. - The bzip2 near-miss was five bytes, so the LENGTH guard refused it and the block-magic check was never reached; removing that check left the suite green. The new sibling is long enough to reach it and NUL-padded so the stdlib still says octet-stream — without the padding the body sniffs as text and the magic table is never consulted at all, which the first attempt at this test demonstrated by passing for the wrong reason. - The ADTS octet-stream gate had no discriminating input: the two-byte case fails validADTSHeader on length before the gate matters. FF F1 40 41 41 41 41 is a structurally valid ADTS header — sync, layer 00, sampling index 0, frame length 2570 — whose every byte the stdlib reads as text, so it answers text/plain. That is the only shape that separates the gate from the structural check, and with the gate removed the file is stored as audio/aac. Refs: BUG-2963 * test(attachments): fuzz the sniff path, since this change added a parser (BUG-2963) Every other check in this package reads fixed offsets and is bounded by construction. sniffEBMLDocType walks caller-supplied length fields, which is the one shape here that can index out of range or fail to advance, so the question 'does it survive malformed input' deserved an answer from running it rather than from reading it. Asserts the two properties a sniffer owes its caller — it returns, and it does not panic — across sniffOpaqueMagic, sniffEBMLDocType, sniffOggAudio, validADTSHeader, SniffMIME and ValidateUpload. What it returns for nonsense is left to the table tests. Seeds are the real fixtures, each truncated at fourteen offsets, plus the shapes a walk breaks on first: unknown-size elements (the reserved all-ones VINT), a child whose declared size exceeds the data, the invalid all-zero VINT marker, a zero-length child that must still advance, and a truncated Ogg page header. 5.56M executions, no panic and no hang. Also collapses a duplicate fixture loader this file had grown alongside mime_isobmff_test.go's readFixture. Refs: BUG-2963 * fix(attachments): use the real parsers, drop Ogg, and stop overclaiming (BUG-2963) Round 2 found 15 issues, two of them P1, and the important one is not any single check — it is that round 1's answer was wrong in the same WAY round 1's defect was. Round 1 defeated prefix matching with an ELF carrying "ustar" at offset 257. The response was a checksum. Round 2 defeated the checksum with an ELF carrying a CORRECT one. Measured while responding, and it settles the question: archive/tar's own Reader.Next accepts that file too. A 512-byte tar header is exactly those fields, and nothing forbids another format's padding from containing them. The two are not distinguishable at this size by anything in the standard library, so this is a property of the formats and not a defect to fix. Adding a third round of field checks would have been the same mistake a third time. So the change is to the CLAIMS as much as to the code. These checks RECOGNISE a format; they do not establish one. The safety property lives elsewhere and is now stated once, at the top of mime_magic.go: whatever the bytes are, they are stored opaquely, never executed or decompressed here, and served under a reviewed type with nosniff, as an attachment for every type recognised here. The checksum-correct ELF ships as a fixture and a test asserting it is ACCEPTED — a limitation recorded where it cannot be rediscovered as a bug. Where a real parser exists, it is now used: archive/tar and compress/bzip2, which cannot drift from the parsers a consumer would use and handle GNU/pax variants for free. bzip2 keeps its magic check alongside the decode because neither subsumes the other — the magic refuses a five-byte stream the decoder can only call truncated, the decode refuses an empty stream whose combined CRC is wrong. OGG IS REMOVED. The alias was ruled in, and review showed the question it has to answer — is this container audio — cannot be answered from the head of the file: an Ogg with Opus first and VP8 second passes a first-packet codec gate, because Ogg multiplexes and the video pages come later. The gate was also wrong in the other direction, refusing legitimate Skeleton-prefixed audio. Ogg is refused exactly as before this branch; making it work means adding video/ogg as a reviewed entry or demuxing, and neither belongs in a change whose premise is that it adds no new trust. The reasoning is a comment where the next person to reach for an alias will meet it. Other round-2 correctness fixes: a DocType value ends at its first NUL, so a real Matroska padded "matroska\x00junk" is no longer stored as WebM; reserved all-ones EBML IDs are refused rather than acting as zero-length children; FLAC checks the STREAMINFO BODY rather than its declaration; 7z checks that the start header's arithmetic is representable, not only that its CRC agrees; ADTS accounts for the two CRC bytes a protected frame declares. Tests gained the direction they lacked entirely — legal variants that must NOT be refused (an empty bzip2 stream, STREAMINFO carrying the last-block flag, a CRC-protected ADTS frame) — and the truncation loop now asserts refusal below each check's minimum instead of discarding its return value, which is why it had been passing while flac.head512[:8] was accepted. Refs: BUG-2963 * test(attachments): cover the five guards the rebuilt matrix found untested (BUG-2963) Rebuilding the mutation matrix against the round-2 code turned up seven survivors. Six were real gaps; each now has the exact input a review round used, or the smallest one that separates the guard from its neighbours, and two carry a control leg so the negative cannot pass for the wrong reason. - 7z: a start header whose next-header offset is 2^64-1, with a recomputed valid CRC. The CRC proves the bytes are intended, not that they are possible. - FLAC: STREAMINFO declaring a zero sample rate. - bzip2: the head of a 200KB archive, which must be RECOGNISED — truncation is what a 512-byte head of any real archive looks like, and the guard that distinguishes it from a structural error had nothing testing it. - EBML: a real Matroska whose DocType payload is 'matroska\x00junk'; and a header with a reserved all-ones ID before a valid DocType. The seventh is an EQUIVALENT mutant and is recorded as one rather than fixed: lowering validTarHeader's length guard from 512 to 262 changes no outcome, because archive/tar refuses a short block by itself. Measured, then written into the comment so the guard reads as clarifying rather than load-bearing. Refs: BUG-2963 * fix(attachments): delete the bzip2 magic comparison a mutation proved dead (BUG-2963) The check compared the block magic AND decoded, under a comment claiming each caught what the other could not. A repaired mutation run refuted it: with the comparison removed, every input the comment credited it with stopping is still refused — the short ones by the length guard, the rest by the decoder. So the line could not change an outcome while its comment said it did, which is the failure this package's own SafeFallbackExtension comment warns about. Deleted rather than demoted, and the length guard now says what it is for. The mutants that found this had been scoring BUILD-FAIL, which is nothing at all rather than a survivor — repairing them to compile is what surfaced three untested guards, two of them FLAC's STREAMINFO block-size floor and ordering. Refs: BUG-2963 * fix(attachments): recognise by magic, to the standard this door already uses (BUG-2963) Three review rounds walked the structural-validation path to its end and the ruling is to stop: - Round 1 defeated magic matching with a real, executing ELF carrying "ustar" at offset 257. - Round 2 defeated the checksum that answered round 1, with an ELF carrying a CORRECT one — and archive/tar's own Reader.Next accepts that file too. A 512-byte tar header is exactly those fields; the two are not distinguishable at this size by anything in the standard library. - Round 3 found the accumulated validation refusing REAL files: PAX and long-name GNU tars, legal randomized bzip2 blocks, FLAC declaring the zero sample rate RFC 9639 permits. That is this bug's own defect — refusing files people legitimately have — reintroduced by the fix for it. Validation could not narrow what the door accepts and had started refusing what it should take, so it is gone. Recognition is by defining magic: ustar at 257, BZh plus its digit, the 7z six bytes, fLaC, and for ADTS the syncword plus the layer bits, which keeps its .aac extension gate because fourteen bits is weaker than the rest. The fact that settles the width, read from the stdlib rather than assumed: http.DetectContentType recognises audio/mpeg from the three bytes "ID3" (net/http/sniff.go), audio/mpeg is on the allowlist, and this door already serves it inline. Every signature here is at least as wide, so this is the door's EXISTING standard rather than a relaxation of it. A test asserts that premise so it cannot rot. Two refusals are deliberate and say so in the code: V7 tar has no magic anywhere and cannot be recognised by this kind of check at all, and Ogg stays refused for the reason a previous commit records. bzip2 is no longer decompressed anywhere in the door. The safety paragraph is rewritten from what the code does, for the third time and the last: nothing is executed, nothing is decompressed, archives download while FLAC and AAC play inline exactly as every other allowlisted audio type does, and nosniff means recognition can move a file between reviewed types but never outside them. Tests now assert the contract that exists. The widening is asserted rather than described — every input the validation used to refuse is accepted, each with its SERVING BUCKET checked, because that is the property that makes it tolerable. The files validation used to refuse are asserted accepted. What remains of the negatives is the only thing still true: the magic has to be there, in the right place, in full. Refs: BUG-2963 * test(attachments): restore two EBML properties the bulk cut dropped (BUG-2963) Cutting the structural-validation tests wholesale took the DocType NUL-termination and reserved-ID cases with them, though the DocType walk they cover is untouched by the magic-only ruling. A mutation run is what noticed: both mutations had gone from detected to surviving. Worth recording as a shape rather than a slip — deleting a test file's worth of obsolete assertions is exactly when live coverage leaves with them, and the matrix is the only thing that says so. Refs: BUG-2963 * fix(attachments): tar wins a prefix collision, and the AAC gate stops refusing real files (BUG-2963) Round 4's two blocking findings were both REAL files of listed types turned away — the direction the ruling's convergence bar names first. **A tar whose first member is named fLaC.txt was refused.** Every recogniser except tar's is a prefix test, and a tar header's first 100 bytes are its member's FILENAME — arbitrary text a user chooses. So an ordinary archive carrying "fLaC" or "BZh9" in a name was recognised as that format and then refused for a category mismatch against its own .tar extension. Tar is tested first now, and the order is load-bearing rather than arbitrary: the collision is asymmetric. A real tar carrying a foreign prefix needs only a filename; a real FLAC carrying "ustar" needs those five bytes at exactly offset 257 in compressed data. Losing the first case costs ordinary uploads. **Real AAC files were refused when their leading bytes looked textual.** The gate ran on application/octet-stream alone, but a raw AAC frame whose ancillary payload is printable makes the first 512 bytes read as text, so the stdlib answers text/plain and a genuine, ffmpeg-decodable .aac was rejected for a category mismatch. Neither verdict is a format detection; both mean "nothing here identifies this", which is the condition under which a weak signature may speak. A type the stdlib DOES recognise is still untouched, and a test pins that with PNG bytes named .aac. Also, the leftovers that keep being mine: comments still describing structural validation that is gone, a test comment claiming production delegates to archive/tar when it no longer does, a README row saying the ELF fixture exists to be refused when it is now accepted, an unused fixture, and two fixtures the README never listed. Tests strengthened where a review round showed one example was standing in for a whole signature: the EBML legs now ask sniffEBMLDocType directly, because routed through SniffMIME the stdlib fallback supplied the same WebM answer and the explicit mapping could be deleted with the suite green; the ADTS signature is walked byte by byte; the bzip2 digit range is tested at both bounds and the FLAC marker at its width and case. Refs: BUG-2963 * test(attachments): give the ADTS verdict gate a control that actually controls (BUG-2963) The PNG leg could not establish the gate it was named for: PNG bytes fail validADTSHeader on the first byte, so they are refused with the gate removed too. A mutation run said so — dropping the stdlib-verdict gate survived the whole suite. The only shape that separates the verdict gate from the structural check is an input that PASSES validADTSHeader and is ALSO identified as something else: a buffer opening with a valid ADTS header and carrying 'ustar' at offset 257, named .aac. It is identified as a tar and must be refused for the category mismatch it is; without the gate the AAC branch overwrites that and accepts it. Recording the process failure alongside it, because it is one I have written down before: this test was lost once between writing and committing, because the mutation runner's restore is 'git checkout -- internal/attachments/' and the work was still uncommitted. Committing is step one of running a control, not step one of the unit. Refs: BUG-2963 * fix(attachments): let the extension arbitrate a magic collision, both ways (BUG-2963) Round 5 found the mirror image of round 4's finding, which is the useful part: my fix for round 4 created it. Round 4 showed a tar whose first member is named fLaC.txt being refused, so tar was ordered first. Round 5 then built complete, decodable FLAC and AAC files carrying "ustar" at offset 257 in ordinary metadata — a Vorbis COMMENT tag is arbitrary UTF-8 (RFC 9639 §8.6) — and those were refused instead. The premise I wrote into the ordering comment was false. I argued the collision was asymmetric, that real audio could not plausibly carry "ustar" at a fixed offset. It can, in a tag a user typed. Any total order refuses somebody. So there is no winner by order. sniffOpaqueCandidates returns EVERY matching type, and when the filename's extension names one of them it breaks the tie. That is a narrower thing than extension trust and the code says so: every candidate is a type the BYTES already matched, so the extension chooses among readings rather than casting a vote, and a name for a type whose magic is absent can never appear in the list. An extension naming none of them changes nothing — asserted, including that a .zip name does not make colliding bytes a zip. The AAC branch is fixed by the same finding from the other direction: it was gated on the REFINED sniff, so a real AAC with "ustar" in its payload — which this package refines to application/x-tar — was refused under its own .aac name. The gate now reads the standard library's verdict, which for those bytes is "nothing identifies this". Kept rather than dropped as unfalsifiable, because no mimesniff signature begins with 0xFF today and this is what stops a fourteen-bit match overriding one that does. Fixtures: the FLAC is real and decodes under libsndfile. The AAC counterpart is NOT shipped — overwriting a real frame's bytes produces a file ffmpeg rejects, so the test uses a synthetic buffer that reproduces the condition and says so rather than claiming to be audio. Also cleared, and reliably my own: comments still describing structural validation, a test's why-strings still citing checksums and CRCs, a comment claiming PAX and GNU archives are refused when they are recognised, and a README gap note that no longer covered all the fixtures. Two test premises tightened where they proved less than they said: the bzip2 prefix case also failed the digit check, and the three-byte ID3 claim was tested with seven. Refs: BUG-2963 * test(attachments): pin the default candidate order (BUG-2963) Reordering sniffOpaqueCandidates changed no test, because extension arbitration settles both known collisions whichever way the list runs. The order still decides one case — colliding bytes whose extension names neither candidate — and nothing asserted it, so the rationale in the comment was unenforced. Refs: BUG-2963 * docs(attachments): the fuzz comment still named the structural validators (BUG-2963) They were removed by the magic-only ruling. Comment-only. Refs: BUG-2963
This commit is contained in:
+117
-18
@@ -109,8 +109,8 @@ var allowed = func() map[string]MIMEEntry {
|
|||||||
// --- Text & data (chip with download) ---
|
// --- Text & data (chip with download) ---
|
||||||
for _, t := range []string{
|
for _, t := range []string{
|
||||||
"text/plain", "text/markdown", "text/csv", "text/tab-separated-values",
|
"text/plain", "text/markdown", "text/csv", "text/tab-separated-values",
|
||||||
"application/json", "application/xml", "text/xml",
|
"application/json", "text/xml",
|
||||||
"application/yaml", "text/yaml", "application/toml",
|
"application/yaml", "application/toml",
|
||||||
} {
|
} {
|
||||||
add(t, RenderChip, CategoryText)
|
add(t, RenderChip, CategoryText)
|
||||||
}
|
}
|
||||||
@@ -124,8 +124,15 @@ var allowed = func() map[string]MIMEEntry {
|
|||||||
}
|
}
|
||||||
|
|
||||||
// --- Forced-download text payloads — would XSS if served inline ---
|
// --- Forced-download text payloads — would XSS if served inline ---
|
||||||
|
// application/javascript was removed here (BUG-2963 F6): no extension in
|
||||||
|
// extMIMEMap reaches that spelling and SniffMIME cannot emit it, so the
|
||||||
|
// entry could never be the type an upload was stored under. text/javascript
|
||||||
|
// stays because .js maps to it — but note it is not reachable EITHER: a .js
|
||||||
|
// upload sniffs text/plain and is stored as that. The difference is that
|
||||||
|
// text/javascript has a route to become reachable (the F5 extension-trust
|
||||||
|
// work) and application/javascript has none, since nothing names it.
|
||||||
for _, t := range []string{
|
for _, t := range []string{
|
||||||
"text/html", "text/javascript", "application/javascript",
|
"text/html", "text/javascript",
|
||||||
} {
|
} {
|
||||||
add(t, RenderForceDownload, CategoryText)
|
add(t, RenderForceDownload, CategoryText)
|
||||||
}
|
}
|
||||||
@@ -156,8 +163,10 @@ func LookupMIME(mime string) (MIMEEntry, bool) {
|
|||||||
// <img>/<audio>/<video>), plus PDF (sandboxed viewer) and plain text. This is
|
// <img>/<audio>/<video>), plus PDF (sandboxed viewer) and plain text. This is
|
||||||
// the server mirror of the client's VIEWER_MIMES + BROWSER_PREVIEW_MIMES
|
// the server mirror of the client's VIEWER_MIMES + BROWSER_PREVIEW_MIMES
|
||||||
// (web/src/lib/attachments/display.ts). Notably absent: image/svg+xml and
|
// (web/src/lib/attachments/display.ts). Notably absent: image/svg+xml and
|
||||||
// application/xhtml+xml (active), text/xml + application/xml (SVG/XHTML wear
|
// application/xhtml+xml (active), text/xml (SVG and XHTML wear it after an
|
||||||
// these after an extensionless sniff), and the whole RenderForceDownload bucket.
|
// extensionless sniff), and the whole RenderForceDownload bucket. The
|
||||||
|
// application/xml spelling used to be named here alongside text/xml; it left
|
||||||
|
// the allowlist entirely in BUG-2963 F6, so there is no entry left to exclude.
|
||||||
var inlineSafe = map[string]struct{}{
|
var inlineSafe = map[string]struct{}{
|
||||||
// Raster images.
|
// Raster images.
|
||||||
"image/png": {}, "image/jpeg": {}, "image/gif": {}, "image/webp": {},
|
"image/png": {}, "image/jpeg": {}, "image/gif": {}, "image/webp": {},
|
||||||
@@ -206,6 +215,12 @@ func NormalizeMIME(mime string) string {
|
|||||||
var sniffAliases = map[string]string{
|
var sniffAliases = map[string]string{
|
||||||
"audio/wave": "audio/wav", // .wav
|
"audio/wave": "audio/wav", // .wav
|
||||||
"application/x-gzip": "application/gzip", // .gz / .tar.gz
|
"application/x-gzip": "application/gzip", // .gz / .tar.gz
|
||||||
|
"video/avi": "video/x-msvideo", // .avi — pure spelling difference
|
||||||
|
// application/ogg is deliberately NOT here, and there is no per-codec
|
||||||
|
// refinement either — both were written and both were removed. An alias
|
||||||
|
// table is for two names of ONE thing, and Ogg is one name for several:
|
||||||
|
// see mime_magic.go for why a container name cannot be resolved to an
|
||||||
|
// audio type from the head of the file.
|
||||||
}
|
}
|
||||||
|
|
||||||
// SniffMIME detects the MIME type from the leading bytes of a payload
|
// SniffMIME detects the MIME type from the leading bytes of a payload
|
||||||
@@ -229,7 +244,28 @@ func SniffMIME(head []byte) string {
|
|||||||
}
|
}
|
||||||
got := NormalizeMIME(http.DetectContentType(head))
|
got := NormalizeMIME(http.DetectContentType(head))
|
||||||
if alias, ok := sniffAliases[got]; ok {
|
if alias, ok := sniffAliases[got]; ok {
|
||||||
return alias
|
got = alias
|
||||||
|
}
|
||||||
|
// Two BUG-2963 refinements. (Ogg was a third and was removed; see
|
||||||
|
// mime_magic.go for why a container name cannot be aliased to an audio
|
||||||
|
// type.) Each is keyed on what the stdlib already said, so none can retype
|
||||||
|
// a file the standard library identified. Neither VALIDATES the format —
|
||||||
|
// see mime_magic.go's header for the three review rounds that settled why
|
||||||
|
// recognition here is by magic:
|
||||||
|
//
|
||||||
|
// - video/webm is refined, because the mimesniff table answers it from
|
||||||
|
// the bare EBML magic and cannot tell Matroska from WebM;
|
||||||
|
// - application/octet-stream is the stdlib having NO opinion, which is
|
||||||
|
// the only case where recognising more formats adds anything.
|
||||||
|
switch got {
|
||||||
|
case "video/webm":
|
||||||
|
if mime := sniffEBMLDocType(head); mime != "" {
|
||||||
|
return mime
|
||||||
|
}
|
||||||
|
case "application/octet-stream":
|
||||||
|
if mime := sniffOpaqueMagic(head); mime != "" {
|
||||||
|
return mime
|
||||||
|
}
|
||||||
}
|
}
|
||||||
return got
|
return got
|
||||||
}
|
}
|
||||||
@@ -252,7 +288,58 @@ func SniffMIME(head []byte) string {
|
|||||||
// extOverride lets callers (the multipart handler) pass the original
|
// extOverride lets callers (the multipart handler) pass the original
|
||||||
// filename so we can compare extensions; pass empty string to skip.
|
// filename so we can compare extensions; pass empty string to skip.
|
||||||
func ValidateUpload(head []byte, filename string) (entry MIMEEntry, code string, err error) {
|
func ValidateUpload(head []byte, filename string) (entry MIMEEntry, code string, err error) {
|
||||||
|
stdlib := NormalizeMIME(http.DetectContentType(head))
|
||||||
|
if alias, ok := sniffAliases[stdlib]; ok {
|
||||||
|
stdlib = alias
|
||||||
|
}
|
||||||
sniffed := SniffMIME(head)
|
sniffed := SniffMIME(head)
|
||||||
|
ext := strings.ToLower(filepath.Ext(filename))
|
||||||
|
|
||||||
|
// More than one magic can match the same bytes — a tar whose first member
|
||||||
|
// is named "fLaC.txt", a FLAC whose COMMENT tag contains "ustar". When the
|
||||||
|
// extension names one of the matching candidates, it breaks the tie. It
|
||||||
|
// cannot introduce a type: every candidate is one the bytes matched, and a
|
||||||
|
// name for a type whose magic is absent never appears in the list.
|
||||||
|
if stdlib == "application/octet-stream" {
|
||||||
|
if alt := preferCandidateForExt(sniffOpaqueCandidates(head), ext); alt != "" {
|
||||||
|
sniffed = alt
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// Raw AAC is the one BUG-2963 format whose structure is too small to act
|
||||||
|
// on from the bytes alone, so it is resolved here — where the filename is
|
||||||
|
// known — rather than inside SniffMIME, which deliberately never sees a
|
||||||
|
// filename. Both halves are required, and neither is sufficient: bytes
|
||||||
|
// without the extension name nothing, and the extension without a valid
|
||||||
|
// ADTS header names nothing either. The extension does not supply
|
||||||
|
// evidence; it decides whether a weak structure may speak.
|
||||||
|
//
|
||||||
|
// What it does NOT do is decide the outcome for a .aac generally. A .aac
|
||||||
|
// whose bytes are some other allowlisted audio type is still stored as
|
||||||
|
// that type by the ordinary rules — the categories agree, so nothing here
|
||||||
|
// refuses it. This branch adds one reading; it removes none.
|
||||||
|
// The gate is on THE STANDARD LIBRARY'S verdict, not on the refined one,
|
||||||
|
// and the difference is not academic: a real AAC frame whose ancillary
|
||||||
|
// payload contains "ustar" at offset 257 is refined to application/x-tar
|
||||||
|
// by this package, and gating on the refined value refused it under its
|
||||||
|
// own .aac name. The stdlib said octet-stream about that file — nothing
|
||||||
|
// identified it — which is the condition under which a weak signature may
|
||||||
|
// speak.
|
||||||
|
//
|
||||||
|
// Both no-opinion verdicts count. text/plain is the second: an AAC frame
|
||||||
|
// whose ancillary payload is printable makes the leading bytes look
|
||||||
|
// textual, and leaving that verdict out refused real files too.
|
||||||
|
//
|
||||||
|
// A file the stdlib DOES identify is untouched. That the gate can fire at
|
||||||
|
// all is a property worth keeping rather than a formality: no signature in
|
||||||
|
// the mimesniff table begins with 0xFF today, so nothing the stdlib names
|
||||||
|
// can pass validADTSHeader — but if one ever does, this gate is what stops
|
||||||
|
// a fourteen-bit match from overriding it.
|
||||||
|
if (stdlib == "application/octet-stream" || stdlib == "text/plain") &&
|
||||||
|
validADTSHeader(head) && ext == ".aac" {
|
||||||
|
sniffed = "audio/aac"
|
||||||
|
}
|
||||||
|
|
||||||
e, ok := LookupMIME(sniffed)
|
e, ok := LookupMIME(sniffed)
|
||||||
if !ok {
|
if !ok {
|
||||||
return MIMEEntry{}, "mime_not_allowed", &uploadError{msg: "MIME type not allowed: " + sniffed}
|
return MIMEEntry{}, "mime_not_allowed", &uploadError{msg: "MIME type not allowed: " + sniffed}
|
||||||
@@ -327,8 +414,10 @@ var canonicalExtForMIME = buildCanonicalExtForMIME()
|
|||||||
//
|
//
|
||||||
// Every key MUST be a value that extMIMEMap actually uses, or the entry is a
|
// Every key MUST be a value that extMIMEMap actually uses, or the entry is a
|
||||||
// line that cannot fire. One of them was exactly that on first writing —
|
// line that cannot fire. One of them was exactly that on first writing —
|
||||||
// "text/yaml", where this map says application/yaml — so the preference never
|
// "text/yaml", where the forward map says application/yaml — so the preference
|
||||||
// applied and .yaml won on length. The test asserts the property rather than
|
// never applied and .yaml won on length. (text/yaml is no longer even on the
|
||||||
|
// allowlist; BUG-2963 F6 removed it as unreachable. The lesson it taught this
|
||||||
|
// map is why the note survives it.) The test asserts the property rather than
|
||||||
// trusting the next reader to notice.
|
// trusting the next reader to notice.
|
||||||
func preferredExtensions() map[string]string {
|
func preferredExtensions() map[string]string {
|
||||||
return map[string]string{
|
return map[string]string{
|
||||||
@@ -345,11 +434,19 @@ func preferredExtensions() map[string]string {
|
|||||||
// aliasExtensions covers allowed MIME spellings that NO extension in
|
// aliasExtensions covers allowed MIME spellings that NO extension in
|
||||||
// extMIMEMap maps to, so reversing the forward table alone leaves them
|
// extMIMEMap maps to, so reversing the forward table alone leaves them
|
||||||
// without an extension: the forward table picks one spelling per extension
|
// without an extension: the forward table picks one spelling per extension
|
||||||
// (.xml says application/xml, .js says text/javascript, .webm says
|
// (.webm says video/webm), while the allowlist accepts the alias spellings
|
||||||
// video/webm), while the allowlist accepts the alias spellings too. An
|
// too. An attachment stored under an alias type with an unstorable filename
|
||||||
// attachment stored under an alias type with an unstorable filename came out
|
// came out of `pad attachment view` extensionless — the exact failure the
|
||||||
// of `pad attachment view` extensionless — the exact failure the delegation
|
// delegation to this package was built to end (codex closing round).
|
||||||
// to this package was built to end (codex closing round).
|
//
|
||||||
|
// This table held four entries until BUG-2963 F6. Three stopped being
|
||||||
|
// alias-shaped for two different reasons, and the distinction is the thing to
|
||||||
|
// keep: text/yaml and application/javascript were REMOVED from the allowlist
|
||||||
|
// as unreachable spellings, so an alias for them would name a refused type;
|
||||||
|
// text/xml is still very much allowed, but the forward map now spells .xml
|
||||||
|
// with it, so the reverse mapping is derived and an alias entry here would be
|
||||||
|
// a duplicate the hygiene assertion below rejects. An entry leaving this
|
||||||
|
// table therefore says nothing on its own about whether the type survived.
|
||||||
//
|
//
|
||||||
// Every key MUST be an allowed type with no forward-derived reverse mapping,
|
// Every key MUST be an allowed type with no forward-derived reverse mapping,
|
||||||
// and every value MUST be an extension the forward map sends to an ALLOWED
|
// and every value MUST be an extension the forward map sends to an ALLOWED
|
||||||
@@ -359,10 +456,7 @@ func preferredExtensions() map[string]string {
|
|||||||
// every allowed MIME type has a reverse extension.
|
// every allowed MIME type has a reverse extension.
|
||||||
func aliasExtensions() map[string]string {
|
func aliasExtensions() map[string]string {
|
||||||
return map[string]string{
|
return map[string]string{
|
||||||
"text/xml": ".xml", // forward map spells it application/xml
|
"audio/webm": ".webm", // forward map spells it video/webm; the container is the same
|
||||||
"text/yaml": ".yml", // forward map spells it application/yaml; .yml matches its preference
|
|
||||||
"application/javascript": ".js", // forward map spells it text/javascript
|
|
||||||
"audio/webm": ".webm", // forward map spells it video/webm; the container is the same
|
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -486,7 +580,12 @@ var extMIMEMap = map[string]string{
|
|||||||
".csv": "text/csv",
|
".csv": "text/csv",
|
||||||
".tsv": "text/tab-separated-values",
|
".tsv": "text/tab-separated-values",
|
||||||
".json": "application/json",
|
".json": "application/json",
|
||||||
".xml": "application/xml",
|
// text/xml, not application/xml: the latter left the allowlist in
|
||||||
|
// BUG-2963 F6, and this map's values are looked up in `allowed` — an
|
||||||
|
// extension pointing at a removed spelling would make every .xml upload
|
||||||
|
// fail extension_blocked, which is the same map's mechanism for refusing
|
||||||
|
// .svg and .exe.
|
||||||
|
".xml": "text/xml",
|
||||||
".yaml": "application/yaml",
|
".yaml": "application/yaml",
|
||||||
".yml": "application/yaml",
|
".yml": "application/yaml",
|
||||||
".toml": "application/toml",
|
".toml": "application/toml",
|
||||||
|
|||||||
@@ -0,0 +1,57 @@
|
|||||||
|
package attachments
|
||||||
|
|
||||||
|
import "testing"
|
||||||
|
|
||||||
|
// FuzzSniffMIME exercises the whole sniff path — the stdlib call, both
|
||||||
|
// refinements, and the magic recognisers — against arbitrary bytes.
|
||||||
|
//
|
||||||
|
// It exists because BUG-2963 added a PARSER. Every other check in this package
|
||||||
|
// reads fixed offsets and is bounded by construction; sniffEBMLDocType walks
|
||||||
|
// caller-supplied length fields, which is the one shape here that can index
|
||||||
|
// out of range or fail to advance. The properties asserted are the two a
|
||||||
|
// sniffer owes its caller: it returns, and it does not panic. What it returns
|
||||||
|
// for nonsense is not asserted — that is the table tests' job.
|
||||||
|
//
|
||||||
|
// Seeds are the real fixtures plus the shapes a walk is most likely to break
|
||||||
|
// on: truncation at every element boundary, declared sizes larger than the
|
||||||
|
// data, and the reserved all-ones VINT that means "unknown size".
|
||||||
|
func FuzzSniffMIME(f *testing.F) {
|
||||||
|
for _, name := range []string{
|
||||||
|
"matroska.head512", "webm.head512", "webm-void-says-matroska.head512",
|
||||||
|
"matroska-void-padded.head512", "tar.head512", "tar-bmp-firstmember.head512",
|
||||||
|
"sevenzip.head512", "flac.head512", "bzip2.head512", "aac-adts.head512",
|
||||||
|
"ogg-opus.head512", "ogg-vp8-video.head512", "avi.head512",
|
||||||
|
"elf-with-ustar-magic.head512",
|
||||||
|
} {
|
||||||
|
b, err := fixtureBytes(name)
|
||||||
|
if err != nil {
|
||||||
|
f.Fatalf("seed %s: %v", name, err)
|
||||||
|
}
|
||||||
|
f.Add(b)
|
||||||
|
// Truncations: the walk must survive running out mid-element.
|
||||||
|
for _, n := range []int{0, 1, 2, 3, 4, 5, 6, 7, 8, 12, 24, 40, 64, 100} {
|
||||||
|
if n < len(b) {
|
||||||
|
f.Add(b[:n])
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
// EBML magic followed by hostile length fields.
|
||||||
|
f.Add([]byte{0x1A, 0x45, 0xDF, 0xA3, 0xFF}) // unknown-size header
|
||||||
|
f.Add([]byte{0x1A, 0x45, 0xDF, 0xA3, 0xFE, 0x42, 0x82, 0xFF}) // unknown-size child
|
||||||
|
f.Add([]byte{0x1A, 0x45, 0xDF, 0xA3, 0xA3, 0x42, 0x82, 0x88, 'm', 'a', 't', 'r'}) // child longer than the data
|
||||||
|
f.Add([]byte{0x1A, 0x45, 0xDF, 0xA3, 0xA3, 0x00, 0x00}) // invalid all-zero VINT marker
|
||||||
|
f.Add([]byte{0x1A, 0x45, 0xDF, 0xA3, 0xA3, 0x80, 0x80}) // zero-length child, must still advance
|
||||||
|
f.Add([]byte("OggS\x00\x02")) // truncated Ogg page header
|
||||||
|
|
||||||
|
f.Fuzz(func(t *testing.T, head []byte) {
|
||||||
|
// Each is called directly as well as through SniffMIME, so a panic is
|
||||||
|
// attributed to the check that owns it rather than to the dispatcher.
|
||||||
|
sniffOpaqueMagic(head)
|
||||||
|
sniffEBMLDocType(head)
|
||||||
|
validADTSHeader(head)
|
||||||
|
SniffMIME(head)
|
||||||
|
ValidateUpload(head, "fuzz.aac")
|
||||||
|
ValidateUpload(head, "fuzz.mkv")
|
||||||
|
ValidateUpload(head, "fuzz.tar")
|
||||||
|
})
|
||||||
|
}
|
||||||
@@ -0,0 +1,372 @@
|
|||||||
|
package attachments
|
||||||
|
|
||||||
|
import (
|
||||||
|
"bytes"
|
||||||
|
"strings"
|
||||||
|
)
|
||||||
|
|
||||||
|
// This file closes the second half of the gap BUG-2961 opened: the upload
|
||||||
|
// allowlist names types that SniffMIME cannot produce, so the door refuses —
|
||||||
|
// or silently retypes — files the allowlist says are supported. A measurement
|
||||||
|
// over real files (BUG-2963) found 25 of 48 allowlist entries unreachable as a
|
||||||
|
// stored type.
|
||||||
|
//
|
||||||
|
// WHAT THESE CHECKS DO. They RECOGNISE a format from its defining magic, to
|
||||||
|
// the same standard the standard library's own detector uses. They do not
|
||||||
|
// prove the bytes are that format, and three review rounds established that
|
||||||
|
// nothing available here could:
|
||||||
|
//
|
||||||
|
// - Round 1 defeated magic-only matching with a real, executing ELF binary
|
||||||
|
// carrying "ustar" at offset 257, stored as application/x-tar.
|
||||||
|
// - Round 2 defeated the checksum that was round 1's answer, with an ELF
|
||||||
|
// carrying a CORRECT one — and archive/tar's own Reader.Next accepts that
|
||||||
|
// file too. A 512-byte tar header is exactly those fields, so the two are
|
||||||
|
// not distinguishable at this size by anything in the standard library.
|
||||||
|
// - Round 3 found the accumulated validation refusing REAL files: PAX and
|
||||||
|
// long-name GNU tars, legal randomized bzip2 blocks, FLAC declaring the
|
||||||
|
// zero sample rate RFC 9639 permits. That is this bug's own defect —
|
||||||
|
// refusing files people legitimately have — reintroduced by the fix.
|
||||||
|
//
|
||||||
|
// So the structural validators are gone and recognition is by magic. The
|
||||||
|
// settling fact is that this is the door's EXISTING standard, not a relaxation
|
||||||
|
// of it: http.DetectContentType recognises audio/mpeg from the three bytes
|
||||||
|
// "ID3" (net/http/sniff.go), audio/mpeg is on the allowlist, and this door
|
||||||
|
// already serves it inline. Every signature here is at least as wide.
|
||||||
|
//
|
||||||
|
// THE SAFETY PROPERTY, written from what the code does — two earlier versions
|
||||||
|
// of this paragraph were checked and found factually wrong:
|
||||||
|
//
|
||||||
|
// - Nothing here is executed, and nothing is decompressed. An earlier
|
||||||
|
// version decoded bzip2 while sniffing; that is gone.
|
||||||
|
// - Serving is per allowlist entry, not uniform. Archives are RenderChip and
|
||||||
|
// absent from inlineSafe, so they download. audio/flac and audio/aac are
|
||||||
|
// inlineSafe like every other allowlisted audio type and play through an
|
||||||
|
// <audio> element, which renders without executing embedded script.
|
||||||
|
// - nosniff is set, so a browser will not re-interpret the bytes as
|
||||||
|
// something more dangerous than the type stored, and that type is on a
|
||||||
|
// reviewed allowlist. Recognising a format wrongly moves a file between
|
||||||
|
// reviewed types; it cannot move it outside them.
|
||||||
|
//
|
||||||
|
// No filename is trusted to introduce a type here. The one place an extension
|
||||||
|
// participates is documented at validADTSHeader, and it only DISAMBIGUATES a
|
||||||
|
// structure the bytes must already carry.
|
||||||
|
|
||||||
|
// sniffOpaqueMagic recognises formats the WHATWG mimesniff table has no
|
||||||
|
// signature for, so http.DetectContentType answers application/octet-stream
|
||||||
|
// for a real file of them. It returns the DEFAULT candidate; see
|
||||||
|
// sniffOpaqueCandidates for why there can be more than one.
|
||||||
|
//
|
||||||
|
// Called ONLY when the stdlib returned application/octet-stream, so it cannot
|
||||||
|
// override a type the standard library identified.
|
||||||
|
func sniffOpaqueMagic(head []byte) string {
|
||||||
|
if c := sniffOpaqueCandidates(head); len(c) > 0 {
|
||||||
|
return c[0]
|
||||||
|
}
|
||||||
|
return ""
|
||||||
|
}
|
||||||
|
|
||||||
|
// sniffOpaqueCandidates returns EVERY type whose magic matches, because more
|
||||||
|
// than one can, and which is right is not always decidable from the bytes.
|
||||||
|
//
|
||||||
|
// The collision is real in both directions and neither side is exotic:
|
||||||
|
//
|
||||||
|
// - A tar header's first 100 bytes are its member's FILENAME, so an ordinary
|
||||||
|
// archive whose first member is called "fLaC.txt" carries the FLAC marker
|
||||||
|
// at offset zero.
|
||||||
|
// - A FLAC file's Vorbis COMMENT tags are arbitrary UTF-8 (RFC 9639 §8.6),
|
||||||
|
// and an AAC frame's ancillary payload is arbitrary bytes, so either can
|
||||||
|
// contain "ustar" at offset 257.
|
||||||
|
//
|
||||||
|
// An earlier version picked a winner by ordering and argued the collision was
|
||||||
|
// asymmetric — that real audio could not plausibly carry "ustar" at a fixed
|
||||||
|
// offset. Review refuted that with complete, decodable FLAC and AAC files
|
||||||
|
// carrying it in ordinary metadata. Any total order refuses somebody: tar
|
||||||
|
// first refuses those, prefixes first refuse the archive.
|
||||||
|
//
|
||||||
|
// So the order here is only a DEFAULT. ValidateUpload, which knows the
|
||||||
|
// filename, may pick a different candidate from this list — see
|
||||||
|
// preferCandidateForExt, and note what that is and is not: the extension
|
||||||
|
// cannot introduce a type, only choose among readings the bytes themselves
|
||||||
|
// support.
|
||||||
|
func sniffOpaqueCandidates(head []byte) []string {
|
||||||
|
var out []string
|
||||||
|
// tar leads the default order because its magic sits at a fixed offset
|
||||||
|
// rather than at a prefix, so it is the one least likely to be an
|
||||||
|
// accident of some other format's leading bytes.
|
||||||
|
if validTarHeader(head) {
|
||||||
|
out = append(out, "application/x-tar")
|
||||||
|
}
|
||||||
|
if validSevenZipHeader(head) {
|
||||||
|
out = append(out, "application/x-7z-compressed")
|
||||||
|
}
|
||||||
|
if validFLACStream(head) {
|
||||||
|
out = append(out, "audio/flac")
|
||||||
|
}
|
||||||
|
if validBzip2Stream(head) {
|
||||||
|
out = append(out, "application/x-bzip2")
|
||||||
|
}
|
||||||
|
return out
|
||||||
|
}
|
||||||
|
|
||||||
|
// preferCandidateForExt returns the candidate that the filename's extension
|
||||||
|
// names, or "" when the extension names none of them.
|
||||||
|
//
|
||||||
|
// This is the ONLY place a filename influences which type is chosen, and the
|
||||||
|
// influence is deliberately weak: every candidate is a type the BYTES already
|
||||||
|
// matched, so the extension breaks a tie rather than casting a vote. A name
|
||||||
|
// that matches nothing in the list changes nothing, and a name for a type
|
||||||
|
// whose magic is absent can never appear in the list at all.
|
||||||
|
func preferCandidateForExt(candidates []string, ext string) string {
|
||||||
|
if len(candidates) < 2 || ext == "" {
|
||||||
|
return ""
|
||||||
|
}
|
||||||
|
want, ok := extMIMEMap[strings.ToLower(ext)]
|
||||||
|
if !ok {
|
||||||
|
return ""
|
||||||
|
}
|
||||||
|
want = NormalizeMIME(want)
|
||||||
|
for _, c := range candidates {
|
||||||
|
if c == want {
|
||||||
|
return c
|
||||||
|
}
|
||||||
|
}
|
||||||
|
return ""
|
||||||
|
}
|
||||||
|
|
||||||
|
// validSevenZipHeader recognises a 7z archive by its six-byte signature.
|
||||||
|
//
|
||||||
|
// There was a start-header CRC check here. It is gone with the rest of the
|
||||||
|
// structural validation (see this file's header): it could not narrow what the
|
||||||
|
// door accepts, and its neighbours had begun refusing real archives.
|
||||||
|
func validSevenZipHeader(head []byte) bool {
|
||||||
|
return bytes.HasPrefix(head, []byte{0x37, 0x7A, 0xBC, 0xAF, 0x27, 0x1C})
|
||||||
|
}
|
||||||
|
|
||||||
|
// validFLACStream recognises a native FLAC stream by its four-byte "fLaC"
|
||||||
|
// marker (RFC 9639 §8.1).
|
||||||
|
//
|
||||||
|
// Four bytes is a wider signature than the stdlib uses for the audio type that
|
||||||
|
// is already on this allowlist and already served inline: audio/mpeg is
|
||||||
|
// recognised from the three bytes "ID3". Recognising FLAC the same way is this
|
||||||
|
// door's existing standard, not a departure from it.
|
||||||
|
func validFLACStream(head []byte) bool {
|
||||||
|
return bytes.HasPrefix(head, []byte("fLaC"))
|
||||||
|
}
|
||||||
|
|
||||||
|
// validBzip2Stream recognises a bzip2 stream by "BZh" and its block-size digit.
|
||||||
|
//
|
||||||
|
// It does NOT decompress. An earlier version decoded through the standard
|
||||||
|
// library to reach the stream CRC, which refused legal randomized blocks Go's
|
||||||
|
// decoder does not implement — a real .bz2 rejected for a decoder's missing
|
||||||
|
// feature. Nothing in this door expands an upload now.
|
||||||
|
func validBzip2Stream(head []byte) bool {
|
||||||
|
return len(head) >= 4 && bytes.HasPrefix(head, []byte("BZh")) &&
|
||||||
|
head[3] >= '1' && head[3] <= '9'
|
||||||
|
}
|
||||||
|
|
||||||
|
// validTarHeader recognises a POSIX ustar archive by the magic at offset 257.
|
||||||
|
//
|
||||||
|
// TWO KINDS OF FILE ARE DELIBERATELY NOT RECOGNISED, and both are refusals
|
||||||
|
// rather than oversights:
|
||||||
|
//
|
||||||
|
// - V7 tar, which predates the magic and has no signature anywhere. There is
|
||||||
|
// nothing at a fixed offset to key on, so it cannot be recognised by this
|
||||||
|
// kind of check at all; it stays refused, as it was before BUG-2963.
|
||||||
|
// - Nothing else. PAX and long-name GNU archives lead with a metadata block
|
||||||
|
// rather than a file header, and they ARE recognised, because the magic is
|
||||||
|
// in that block too. A full PARSE would need more blocks than this door
|
||||||
|
// ever reads, which is one of the reasons the parse is gone.
|
||||||
|
//
|
||||||
|
// The checksum that used to be verified here is gone. It never distinguished a
|
||||||
|
// crafted header from a real one — archive/tar accepts an ELF carrying a
|
||||||
|
// correct one, which ships as a fixture — and verifying it required a full
|
||||||
|
// 512-byte block, which is why PAX and long-name GNU archives were refused.
|
||||||
|
func validTarHeader(head []byte) bool {
|
||||||
|
const magicEnd = 262
|
||||||
|
return len(head) >= magicEnd && bytes.Equal(head[257:magicEnd], []byte("ustar"))
|
||||||
|
}
|
||||||
|
|
||||||
|
// sniffEBMLDocType returns the MIME for an EBML file by READING ITS DOCTYPE
|
||||||
|
// ELEMENT, not by searching for a string.
|
||||||
|
//
|
||||||
|
// The mimesniff table maps the bare EBML magic (1A 45 DF A3) to video/webm
|
||||||
|
// with no DocType check, so a Matroska file — on the allowlist under its own
|
||||||
|
// name — is accepted and stored as WebM. That is not a refusal, which is why
|
||||||
|
// it survived a pass looking for 415s: the upload succeeds and the stored type
|
||||||
|
// is simply false.
|
||||||
|
//
|
||||||
|
// This walks the EBML header's child elements. The first version searched the
|
||||||
|
// leading 64 bytes for the two strings instead, and an adversarial round broke
|
||||||
|
// it in BOTH directions with files that ffprobe accepts: a WebM carrying
|
||||||
|
// "matroska" inside a Void element was stored as Matroska, and a Matroska with
|
||||||
|
// forty bytes of legal Void padding — which pushes DocType past 64 — was
|
||||||
|
// stored as WebM, leaving the very defect this fixes live for any file a muxer
|
||||||
|
// chose to pad. Void is legal anywhere and its contents are meaningless, so
|
||||||
|
// only a parse can tell payload from padding.
|
||||||
|
//
|
||||||
|
// Called only when the stdlib returned video/webm, so it can move a file
|
||||||
|
// between the two EBML types and nowhere else. An unreadable or absent DocType
|
||||||
|
// returns "" and the stdlib verdict stands, which is the behaviour that
|
||||||
|
// shipped before this existed.
|
||||||
|
func sniffEBMLDocType(head []byte) string {
|
||||||
|
const ebmlHeaderID = 0x1A45DFA3
|
||||||
|
const docTypeID = 0x4282
|
||||||
|
|
||||||
|
id, rest, ok := readEBMLID(head)
|
||||||
|
if !ok || id != ebmlHeaderID {
|
||||||
|
return ""
|
||||||
|
}
|
||||||
|
size, rest, ok := readEBMLSize(rest)
|
||||||
|
if !ok {
|
||||||
|
return ""
|
||||||
|
}
|
||||||
|
// The header may be longer than the bytes we were given; walking what we
|
||||||
|
// have is correct, and running out simply means no DocType was found.
|
||||||
|
if size < uint64(len(rest)) {
|
||||||
|
rest = rest[:size]
|
||||||
|
}
|
||||||
|
|
||||||
|
for len(rest) > 0 {
|
||||||
|
childID, after, ok := readEBMLID(rest)
|
||||||
|
if !ok {
|
||||||
|
return ""
|
||||||
|
}
|
||||||
|
childSize, after, ok := readEBMLSize(after)
|
||||||
|
if !ok {
|
||||||
|
return ""
|
||||||
|
}
|
||||||
|
if childSize > uint64(len(after)) {
|
||||||
|
// The declared payload runs past what we hold. For DocType that
|
||||||
|
// is still answerable when the VALUE is complete in hand: it ends
|
||||||
|
// at its first NUL, and a header padding DocType out to 600 bytes
|
||||||
|
// puts "matroska\x00" in the first dozen. Anything else, we stop.
|
||||||
|
if childID == docTypeID {
|
||||||
|
if mime := docTypeMIME(after); mime != "" {
|
||||||
|
return mime
|
||||||
|
}
|
||||||
|
}
|
||||||
|
return ""
|
||||||
|
}
|
||||||
|
if childID == docTypeID {
|
||||||
|
// DocType is an ASCII string whose value ENDS at the first NUL;
|
||||||
|
// anything after that is padding and is not part of the value
|
||||||
|
// (RFC 8794 §13). TrimRight was wrong here — a round-2 finding
|
||||||
|
// produced an ffprobe-readable Matroska whose DocType payload was
|
||||||
|
// "matroska\x00junk", which trimming left intact and so failed to
|
||||||
|
// match, storing a real Matroska as WebM.
|
||||||
|
return docTypeMIME(after[:childSize])
|
||||||
|
}
|
||||||
|
rest = after[childSize:]
|
||||||
|
}
|
||||||
|
return ""
|
||||||
|
}
|
||||||
|
|
||||||
|
// readEBMLID reads an EBML element ID. IDs keep their length marker as part of
|
||||||
|
// the value (RFC 8794 §5), so DocType is the four-hex-digit 0x4282 that the
|
||||||
|
// specification names, and the marker byte tells us how many bytes to take.
|
||||||
|
func readEBMLID(b []byte) (id uint32, rest []byte, ok bool) {
|
||||||
|
if len(b) == 0 {
|
||||||
|
return 0, nil, false
|
||||||
|
}
|
||||||
|
n := ebmlLength(b[0])
|
||||||
|
if n == 0 || n > 4 || len(b) < n {
|
||||||
|
return 0, nil, false
|
||||||
|
}
|
||||||
|
for i := 0; i < n; i++ {
|
||||||
|
id = id<<8 | uint32(b[i])
|
||||||
|
}
|
||||||
|
// An ID whose value bits are all ones is RESERVED and not a valid element
|
||||||
|
// ID (RFC 8794 §5). Accepting one let malformed bytes act as a
|
||||||
|
// zero-length child and carry the walk onward to a DocType that follows
|
||||||
|
// them, which a round-2 finding used to have 1a45dfa3 8d ff80 ... parse
|
||||||
|
// as Matroska.
|
||||||
|
if id == allOnesEBMLID[n] {
|
||||||
|
return 0, nil, false
|
||||||
|
}
|
||||||
|
return id, b[n:], true
|
||||||
|
}
|
||||||
|
|
||||||
|
// allOnesEBMLID is the reserved value at each ID width: the marker bit plus
|
||||||
|
// every value bit set.
|
||||||
|
var allOnesEBMLID = [5]uint32{0, 0xFF, 0x7FFF, 0x3FFFFF, 0x1FFFFFFF}
|
||||||
|
|
||||||
|
// readEBMLSize reads an EBML data size, whose length marker is REMOVED from
|
||||||
|
// the value (unlike an ID). An all-ones value means "unknown size", which this
|
||||||
|
// treats as unreadable — a header of unknown length is not something to walk.
|
||||||
|
func readEBMLSize(b []byte) (size uint64, rest []byte, ok bool) {
|
||||||
|
if len(b) == 0 {
|
||||||
|
return 0, nil, false
|
||||||
|
}
|
||||||
|
n := ebmlLength(b[0])
|
||||||
|
if n == 0 || n > 8 || len(b) < n {
|
||||||
|
return 0, nil, false
|
||||||
|
}
|
||||||
|
size = uint64(b[0]) & (1<<(8-uint(n)) - 1)
|
||||||
|
allOnes := size == 1<<(8-uint(n))-1
|
||||||
|
for i := 1; i < n; i++ {
|
||||||
|
size = size<<8 | uint64(b[i])
|
||||||
|
allOnes = allOnes && b[i] == 0xFF
|
||||||
|
}
|
||||||
|
if allOnes {
|
||||||
|
return 0, nil, false
|
||||||
|
}
|
||||||
|
return size, b[n:], true
|
||||||
|
}
|
||||||
|
|
||||||
|
// ebmlLength returns how many bytes a VINT starting with this byte occupies —
|
||||||
|
// one plus the number of leading zero bits — or 0 for the invalid all-zero
|
||||||
|
// marker byte.
|
||||||
|
func ebmlLength(first byte) int {
|
||||||
|
for i := 0; i < 8; i++ {
|
||||||
|
if first&(0x80>>uint(i)) != 0 {
|
||||||
|
return i + 1
|
||||||
|
}
|
||||||
|
}
|
||||||
|
return 0
|
||||||
|
}
|
||||||
|
|
||||||
|
// validADTSHeader recognises the ADTS framing raw .aac files use: the 12-bit
|
||||||
|
// syncword plus the two layer bits ADTS requires to be zero.
|
||||||
|
//
|
||||||
|
// THIS SIGNATURE IS WEAKER THAN THE REST, and that is why its caller treats it
|
||||||
|
// differently. Fourteen bits at offset zero is less than the three bytes the
|
||||||
|
// stdlib uses for audio/mpeg, and MPEG audio shares the syncword's shape. So
|
||||||
|
// ValidateUpload also requires the .aac EXTENSION — not as evidence, but to
|
||||||
|
// decide whether a signature this weak may speak at all.
|
||||||
|
//
|
||||||
|
// The frame-length, sampling-index and protection arithmetic that used to be
|
||||||
|
// here is gone with the other structural validation. Review found it wrong in
|
||||||
|
// both directions — accepting frames whose declared length could not hold
|
||||||
|
// their own header, and rejecting legal MPEG-2 profiles — which is the tail
|
||||||
|
// this file's header describes.
|
||||||
|
//
|
||||||
|
// Layout (ISO/IEC 13818-7 §6.2, adts_fixed_header):
|
||||||
|
//
|
||||||
|
// byte 0 syncword high 8 bits
|
||||||
|
// byte 1 sync low 4 | MPEG version | layer (2 bits) | protection absent
|
||||||
|
func validADTSHeader(head []byte) bool {
|
||||||
|
if len(head) < 2 {
|
||||||
|
return false
|
||||||
|
}
|
||||||
|
if head[0] != 0xFF || head[1]&0xF0 != 0xF0 {
|
||||||
|
return false
|
||||||
|
}
|
||||||
|
return head[1]>>1&0x03 == 0 // layer MUST be 00 for ADTS
|
||||||
|
}
|
||||||
|
|
||||||
|
// docTypeMIME maps a DocType payload to a MIME type. The value ENDS at its
|
||||||
|
// first NUL and anything after is padding (RFC 8794 §13); a payload with no
|
||||||
|
// NUL at all is the whole slice. A DocType this does not recognise returns ""
|
||||||
|
// — not ours to name, and the stdlib's answer is no worse than a guess.
|
||||||
|
func docTypeMIME(payload []byte) string {
|
||||||
|
if i := bytes.IndexByte(payload, 0); i >= 0 {
|
||||||
|
payload = payload[:i]
|
||||||
|
}
|
||||||
|
switch string(payload) {
|
||||||
|
case "matroska":
|
||||||
|
return "video/x-matroska"
|
||||||
|
case "webm":
|
||||||
|
return "video/webm"
|
||||||
|
}
|
||||||
|
return ""
|
||||||
|
}
|
||||||
@@ -0,0 +1,673 @@
|
|||||||
|
package attachments
|
||||||
|
|
||||||
|
import (
|
||||||
|
"bytes"
|
||||||
|
"os"
|
||||||
|
"path/filepath"
|
||||||
|
"testing"
|
||||||
|
)
|
||||||
|
|
||||||
|
// fixtureBytes is the testing-free half of this package's fixture loading, so
|
||||||
|
// a *testing.F can build a fuzz seed corpus from the same files that
|
||||||
|
// readFixture (mime_isobmff_test.go) hands the table tests.
|
||||||
|
func fixtureBytes(name string) ([]byte, error) {
|
||||||
|
return os.ReadFile(filepath.Join("testdata", name))
|
||||||
|
}
|
||||||
|
|
||||||
|
// TestBUG2963FormatsReachTheAllowlist is the headline property: for each
|
||||||
|
// format BUG-2963 measured as unreachable, a real file of it now sniffs as the
|
||||||
|
// spelling the allowlist uses and is ACCEPTED and stored under that spelling.
|
||||||
|
//
|
||||||
|
// Both halves are asserted deliberately. The sniff alone is not the claim —
|
||||||
|
// the claim is what the door does — and the two can diverge, which is exactly
|
||||||
|
// how .mkv hid: it sniffed to something the allowlist accepted, so the upload
|
||||||
|
// succeeded while the stored type was false.
|
||||||
|
func TestBUG2963FormatsReachTheAllowlist(t *testing.T) {
|
||||||
|
cases := []struct {
|
||||||
|
fixture string
|
||||||
|
filename string
|
||||||
|
want string // what the door stores
|
||||||
|
// sniffWant is what SniffMIME alone returns, when that differs from
|
||||||
|
// what the door stores. It differs for exactly one format: raw AAC is
|
||||||
|
// resolved in ValidateUpload, because its structure is too small to
|
||||||
|
// act on without the filename and SniffMIME never sees one.
|
||||||
|
sniffWant string
|
||||||
|
cat Category
|
||||||
|
why string
|
||||||
|
}{
|
||||||
|
{"tar.head512", "archive.tar", "application/x-tar", "", CategoryArchive, "ustar at offset 257"},
|
||||||
|
{"bzip2.head512", "notes.txt.bz2", "application/x-bzip2", "", CategoryArchive, "BZh plus the block-size digit"},
|
||||||
|
{"sevenzip.head512", "archive.7z", "application/x-7z-compressed", "", CategoryArchive, "the six-byte signature"},
|
||||||
|
{"flac.head512", "track.flac", "audio/flac", "", CategoryAudio, "fLaC plus a 34-byte STREAMINFO block"},
|
||||||
|
{"aac-adts.head512", "track.aac", "audio/aac", "application/octet-stream", CategoryAudio, "valid ADTS header, gated on the extension"},
|
||||||
|
{"avi.head512", "clip.avi", "video/x-msvideo", "", CategoryVideo, "video/avi alias"},
|
||||||
|
{"matroska.head512", "clip.mkv", "video/x-matroska", "", CategoryVideo, "EBML DocType matroska"},
|
||||||
|
}
|
||||||
|
for _, tc := range cases {
|
||||||
|
t.Run(tc.fixture, func(t *testing.T) {
|
||||||
|
head := readFixture(t, tc.fixture)
|
||||||
|
wantSniff := tc.sniffWant
|
||||||
|
if wantSniff == "" {
|
||||||
|
wantSniff = tc.want
|
||||||
|
}
|
||||||
|
if got := SniffMIME(head); got != wantSniff {
|
||||||
|
t.Errorf("SniffMIME = %q, want %q (%s)", got, wantSniff, tc.why)
|
||||||
|
}
|
||||||
|
entry, code, err := ValidateUpload(head, tc.filename)
|
||||||
|
if err != nil {
|
||||||
|
t.Fatalf("ValidateUpload(%s) rejected: code=%s err=%v", tc.filename, code, err)
|
||||||
|
}
|
||||||
|
if entry.MIME != tc.want {
|
||||||
|
t.Errorf("stored as %q, want %q", entry.MIME, tc.want)
|
||||||
|
}
|
||||||
|
if entry.Category != tc.cat {
|
||||||
|
t.Errorf("category %q, want %q", entry.Category, tc.cat)
|
||||||
|
}
|
||||||
|
})
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// TestEBMLDocTypeIsParsedNotSearched pins the DocType read in all four
|
||||||
|
// directions. The first two are the ordinary files. The last two are the pair
|
||||||
|
// an adversarial round used to break a substring search, and they broke it
|
||||||
|
// BOTH ways — the reason this is a parse now.
|
||||||
|
//
|
||||||
|
// Both adversarial fixtures were built by inserting a legal Void element (ID
|
||||||
|
// 0xEC, contents meaningless by specification) into the ordinary fixtures and
|
||||||
|
// widening the header's size field to match. The COMPLETE files ffprobe reads;
|
||||||
|
// what is committed here is their first 512 bytes, like every fixture in this
|
||||||
|
// package, because that is the whole input domain of SniffMIME. Running
|
||||||
|
// ffprobe on the committed prefix fails with a premature EOF, which says
|
||||||
|
// nothing about the file it came from — an earlier version of this comment
|
||||||
|
// claimed the committed files were themselves readable, and they are not.
|
||||||
|
func TestEBMLDocTypeIsParsedNotSearched(t *testing.T) {
|
||||||
|
cases := []struct {
|
||||||
|
fixture string
|
||||||
|
want string
|
||||||
|
why string
|
||||||
|
}{
|
||||||
|
{"matroska.head512", "video/x-matroska", "the ordinary Matroska file"},
|
||||||
|
{"webm.head512", "video/webm", "the ordinary WebM file — a read that always said Matroska would break this"},
|
||||||
|
{"webm-void-says-matroska.head512", "video/webm",
|
||||||
|
"a WebM whose Void padding contains the string \"matroska\"; its real DocType is webm"},
|
||||||
|
{"matroska-void-padded.head512", "video/x-matroska",
|
||||||
|
"a Matroska whose Void padding pushes DocType to offset 66 — past any fixed leading window"},
|
||||||
|
}
|
||||||
|
for _, tc := range cases {
|
||||||
|
t.Run(tc.fixture, func(t *testing.T) {
|
||||||
|
head := readFixture(t, tc.fixture)
|
||||||
|
// sniffEBMLDocType directly, not through SniffMIME. The WebM legs
|
||||||
|
// are the reason: SniffMIME falls back to the stdlib, which also
|
||||||
|
// answers video/webm, so a test routed through it stayed green
|
||||||
|
// with the explicit webm mapping deleted. Asking the parser is
|
||||||
|
// what makes the parser the thing under test.
|
||||||
|
if got := sniffEBMLDocType(head); got != tc.want {
|
||||||
|
t.Errorf("sniffEBMLDocType = %q, want %q — %s", got, tc.want, tc.why)
|
||||||
|
}
|
||||||
|
if got := SniffMIME(head); got != tc.want {
|
||||||
|
t.Errorf("SniffMIME = %q, want %q — %s", got, tc.want, tc.why)
|
||||||
|
}
|
||||||
|
})
|
||||||
|
}
|
||||||
|
|
||||||
|
// EBML magic, then bytes that parse to no DocType at all. The fallback is
|
||||||
|
// the stdlib's answer, so an unreadable header degrades to the behaviour
|
||||||
|
// that shipped before this check existed rather than to a refusal.
|
||||||
|
blank := append([]byte{0x1A, 0x45, 0xDF, 0xA3}, make([]byte, 120)...)
|
||||||
|
if got := SniffMIME(blank); got != "video/webm" {
|
||||||
|
t.Errorf("EBML header with no DocType sniffed %q, want the stdlib's video/webm", got)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// TestOggStaysRefused records a decision, not a mechanism: Ogg is NOT
|
||||||
|
// recognised, and both fixtures are kept so the next person to reach for an
|
||||||
|
// application/ogg alias meets the evidence first.
|
||||||
|
//
|
||||||
|
// An alias was written, ruled in, and then removed after review showed the
|
||||||
|
// question it has to answer — is this container AUDIO — cannot be answered
|
||||||
|
// from the head of the file. Ogg multiplexes, so a second video stream's pages
|
||||||
|
// come later than any sniff can see. Both files below are refused today
|
||||||
|
// exactly as they were before BUG-2963, which is the point: no regression, and
|
||||||
|
// no acceptance of a type the allowlist never reviewed.
|
||||||
|
func TestOggStaysRefused(t *testing.T) {
|
||||||
|
for _, f := range []string{"ogg-opus.head512", "ogg-vp8-video.head512"} {
|
||||||
|
t.Run(f, func(t *testing.T) {
|
||||||
|
if got := SniffMIME(readFixture(t, f)); got != "application/ogg" {
|
||||||
|
t.Errorf("SniffMIME = %q, want the unaliased application/ogg", got)
|
||||||
|
}
|
||||||
|
if entry, _, err := ValidateUpload(readFixture(t, f), "track.ogg"); err == nil {
|
||||||
|
t.Errorf("accepted and stored as %q; audio/ogg stays unreachable until "+
|
||||||
|
"either video/ogg is a reviewed allowlist entry or something "+
|
||||||
|
"demuxes far enough to enumerate the streams", entry.MIME)
|
||||||
|
}
|
||||||
|
})
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// TestADTSGate covers the one place a filename participates, in both
|
||||||
|
// directions. The gate is on the stdlib having no SPECIFIC format opinion —
|
||||||
|
// application/octet-stream or text/plain — and on the .aac extension. Neither
|
||||||
|
// half is sufficient and the pair is not a formality: review found real,
|
||||||
|
// ffmpeg-decodable AAC files being refused because only the first verdict was
|
||||||
|
// accepted.
|
||||||
|
func TestADTSGate(t *testing.T) {
|
||||||
|
adts := readFixture(t, "aac-adts.head512")
|
||||||
|
|
||||||
|
if _, _, err := ValidateUpload(adts, "track.aac"); err != nil {
|
||||||
|
t.Fatalf("premise failed: the real ADTS fixture named .aac is rejected: %v", err)
|
||||||
|
}
|
||||||
|
|
||||||
|
// Structure without the extension: nothing asked for this reading.
|
||||||
|
if _, code, err := ValidateUpload(adts, "track.bin"); err == nil {
|
||||||
|
t.Error("ADTS bytes named .bin were accepted; the extension gate is not applied")
|
||||||
|
} else if code != "mime_not_allowed" {
|
||||||
|
t.Errorf("code = %q, want mime_not_allowed", code)
|
||||||
|
}
|
||||||
|
|
||||||
|
// Extension without the structure.
|
||||||
|
if _, code, err := ValidateUpload(make([]byte, 512), "track.aac"); err == nil {
|
||||||
|
t.Error("zero bytes named .aac were accepted; the extension is being trusted alone")
|
||||||
|
} else if code != "mime_not_allowed" {
|
||||||
|
t.Errorf("code = %q, want mime_not_allowed", code)
|
||||||
|
}
|
||||||
|
|
||||||
|
// A REAL AAC file can look textual. These seven bytes are a valid ADTS
|
||||||
|
// header whose every byte the stdlib reads as text, so it answers
|
||||||
|
// text/plain — the shape of an AAC frame whose ancillary payload is
|
||||||
|
// printable. Review confirmed ffmpeg decodes such a file. It must be
|
||||||
|
// ACCEPTED: refusing it was a real file of a listed type turned away.
|
||||||
|
textual := []byte{0xFF, 0xF1, 0x40, 0x41, 0x41, 0x41, 0x41}
|
||||||
|
if !validADTSHeader(textual) {
|
||||||
|
t.Fatal("premise failed: the input must be a valid ADTS header")
|
||||||
|
}
|
||||||
|
if got := SniffMIME(textual); got != "text/plain" {
|
||||||
|
t.Fatalf("premise failed: the stdlib called it %q, not text/plain — this case "+
|
||||||
|
"exists to cover the text/plain half of the gate", got)
|
||||||
|
}
|
||||||
|
entry, _, err := ValidateUpload(textual, "textual.aac")
|
||||||
|
if err != nil {
|
||||||
|
t.Errorf("a valid ADTS header the stdlib reads as text was refused: %v", err)
|
||||||
|
} else if entry.MIME != "audio/aac" {
|
||||||
|
t.Errorf("stored as %q, want audio/aac", entry.MIME)
|
||||||
|
}
|
||||||
|
|
||||||
|
// A type that IS identified is untouched by the .aac name. PNG bytes stay
|
||||||
|
// an image — but note what that leg does and does not establish: PNG fails
|
||||||
|
// validADTSHeader on its first byte, so it would be refused with the
|
||||||
|
// verdict gate removed too. It is a sanity case, not a control.
|
||||||
|
if _, code, _ := ValidateUpload([]byte("\x89PNG\r\n\x1a\n"), "sneaky.aac"); code != "mime_extension_mismatch" {
|
||||||
|
t.Errorf("PNG bytes named .aac gave code %q, want mime_extension_mismatch", code)
|
||||||
|
}
|
||||||
|
|
||||||
|
// An AAC whose payload contains "ustar" at offset 257. THIS PACKAGE
|
||||||
|
// refines such bytes to application/x-tar, and gating the AAC branch on
|
||||||
|
// that refined value refused the file under its own .aac name — a review
|
||||||
|
// round built a complete, ffmpeg-decodable AAC of exactly this shape. The
|
||||||
|
// gate reads the STANDARD LIBRARY's verdict instead, which for these bytes
|
||||||
|
// is "nothing identifies this", so the branch runs and the file is
|
||||||
|
// accepted.
|
||||||
|
//
|
||||||
|
// The buffer here is synthetic: it reproduces the CONDITION (a valid ADTS
|
||||||
|
// header, plus our own recogniser answering something else) without
|
||||||
|
// claiming to be decodable audio, since overwriting a real frame's bytes
|
||||||
|
// produces a file ffmpeg rejects. The real-file half of this class is
|
||||||
|
// carried by flac-ustar-in-comment.head512, which does decode.
|
||||||
|
collide := make([]byte, 512)
|
||||||
|
copy(collide, adts[:8])
|
||||||
|
copy(collide[257:], []byte("ustar"))
|
||||||
|
if !validADTSHeader(collide) {
|
||||||
|
t.Fatal("premise failed: the collision buffer must be a valid ADTS header")
|
||||||
|
}
|
||||||
|
if got := SniffMIME(collide); got != "application/x-tar" {
|
||||||
|
t.Fatalf("premise failed: SniffMIME said %q, want application/x-tar — this case "+
|
||||||
|
"exists because our own refinement disagrees with the stdlib here", got)
|
||||||
|
}
|
||||||
|
if entry, _, err := ValidateUpload(collide, "track.aac"); err != nil {
|
||||||
|
t.Errorf("refused (%v); the gate must read the stdlib's verdict, not ours", err)
|
||||||
|
} else if entry.MIME != "audio/aac" {
|
||||||
|
t.Errorf("stored as %q, want audio/aac", entry.MIME)
|
||||||
|
}
|
||||||
|
|
||||||
|
// The signature, byte by byte. Each leg below fails for exactly one
|
||||||
|
// reason, because a single example leaves most of the check untested —
|
||||||
|
// a review round confirmed that dropping the first-byte test, or checking
|
||||||
|
// only one of the two layer bits, left the suite green.
|
||||||
|
for _, tc := range []struct {
|
||||||
|
name string
|
||||||
|
mutate func([]byte)
|
||||||
|
why string
|
||||||
|
}{
|
||||||
|
{"first sync byte wrong", func(b []byte) { b[0] = 0xFE }, "the syncword is 12 bits across two bytes"},
|
||||||
|
{"second sync nibble wrong", func(b []byte) { b[1] &^= 0x10 }, "the high nibble of byte 1 completes the sync"},
|
||||||
|
{"layer bit 1 set", func(b []byte) { b[1] = b[1]&0xF9 | 0x02 }, "ADTS requires layer 00"},
|
||||||
|
{"layer bit 2 set", func(b []byte) { b[1] = b[1]&0xF9 | 0x04 }, "both layer bits are checked, not one"},
|
||||||
|
} {
|
||||||
|
t.Run(tc.name, func(t *testing.T) {
|
||||||
|
b := append([]byte(nil), adts...)
|
||||||
|
tc.mutate(b)
|
||||||
|
if _, _, err := ValidateUpload(b, "track.aac"); err == nil {
|
||||||
|
t.Errorf("accepted — %s", tc.why)
|
||||||
|
}
|
||||||
|
})
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// TestTarWinsAPrefixCollision covers a real archive refused because another
|
||||||
|
// format's magic appeared in its member's FILENAME. A tar header's first 100
|
||||||
|
// bytes are user-chosen text, so any prefix recogniser can collide with an
|
||||||
|
// ordinary archive; the collision is asymmetric, which is why tar is tested
|
||||||
|
// first.
|
||||||
|
func TestTarWinsAPrefixCollision(t *testing.T) {
|
||||||
|
b := readFixture(t, "tar-flac-named-member.head512")
|
||||||
|
if string(b[:4]) != "fLaC" {
|
||||||
|
t.Fatal("premise failed: the fixture's first member must be named fLaC.txt, " +
|
||||||
|
"or there is no collision to arbitrate")
|
||||||
|
}
|
||||||
|
if !bytes.Equal(b[257:262], []byte("ustar")) {
|
||||||
|
t.Fatal("premise failed: the fixture must be a real ustar archive")
|
||||||
|
}
|
||||||
|
entry, code, err := ValidateUpload(b, "archive.tar")
|
||||||
|
if err != nil {
|
||||||
|
t.Fatalf("a real tar whose first member is named fLaC.txt was refused (code=%s)", code)
|
||||||
|
}
|
||||||
|
if entry.MIME != "application/x-tar" {
|
||||||
|
t.Errorf("stored as %q, want application/x-tar", entry.MIME)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// TestOpaqueMagicLosesToTheStdlib pins the ordering: the magic table is
|
||||||
|
// consulted only where the stdlib had no opinion, so it can add a detection
|
||||||
|
// and never replace one.
|
||||||
|
//
|
||||||
|
// The fixture is a REAL tar archive whose first member is named BM.txt, which
|
||||||
|
// makes the stdlib answer image/bmp. That is a genuine limitation of this
|
||||||
|
// design and is asserted rather than hidden: such a tar is refused today and
|
||||||
|
// still is. An earlier version of this test used a spliced PNG and claimed no
|
||||||
|
// real file exercised competing detections, which was false.
|
||||||
|
//
|
||||||
|
// The mutation that kills this test is hoisting sniffOpaqueMagic ABOVE the
|
||||||
|
// stdlib call. Deleting the application/octet-stream case does NOT — that is
|
||||||
|
// caught by the positive tests instead. The distinction is recorded because
|
||||||
|
// the comment here previously named the wrong mutation, contradicting the
|
||||||
|
// project's own matrix.
|
||||||
|
func TestOpaqueMagicLosesToTheStdlib(t *testing.T) {
|
||||||
|
tarBMP := readFixture(t, "tar-bmp-firstmember.head512")
|
||||||
|
|
||||||
|
if !validTarHeader(tarBMP) {
|
||||||
|
t.Fatal("premise failed: the fixture must be recognised as a tar, " +
|
||||||
|
"or this asserts nothing about which check wins")
|
||||||
|
}
|
||||||
|
if got := SniffMIME(tarBMP); got != "image/bmp" {
|
||||||
|
t.Errorf("SniffMIME = %q, want image/bmp — a recognised tar must not "+
|
||||||
|
"override a type the stdlib identified", got)
|
||||||
|
}
|
||||||
|
if _, code, err := ValidateUpload(tarBMP, "archive.tar"); err == nil {
|
||||||
|
t.Error("accepted; a tar the stdlib reads as an image is refused, as it was before this change")
|
||||||
|
} else if code == "" {
|
||||||
|
t.Errorf("rejected with an empty code")
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// TestBUG2963F6RemovedSpellings covers the deletions and, more importantly,
|
||||||
|
// the thing deleting them nearly broke. extMIMEMap's values are looked up in
|
||||||
|
// `allowed`, and an extension whose value is NOT allowed is the mechanism that
|
||||||
|
// refuses .svg and .exe — so removing application/xml while .xml still named
|
||||||
|
// it would have turned every XML upload into extension_blocked. The upload leg
|
||||||
|
// is the regression guard; the lookup legs are the deletion itself.
|
||||||
|
func TestBUG2963F6RemovedSpellings(t *testing.T) {
|
||||||
|
for _, m := range []string{"application/javascript", "text/yaml", "application/xml"} {
|
||||||
|
if _, ok := LookupMIME(m); ok {
|
||||||
|
t.Errorf("%q is still on the allowlist; BUG-2963 F6 removed it as unreachable", m)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
xml := []byte(`<?xml version="1.0" encoding="UTF-8"?><root><item ref="BUG-2963"/></root>`)
|
||||||
|
entry, code, err := ValidateUpload(xml, "data.xml")
|
||||||
|
if err != nil {
|
||||||
|
t.Fatalf(".xml upload rejected after the F6 deletions: code=%s err=%v", code, err)
|
||||||
|
}
|
||||||
|
if entry.MIME != "text/xml" {
|
||||||
|
t.Errorf(".xml stored as %q, want text/xml", entry.MIME)
|
||||||
|
}
|
||||||
|
|
||||||
|
// audio/webm is unreachable too and STAYS, by ruling: separating it from
|
||||||
|
// video/webm needs a track read, nobody has asked, and an unreachable
|
||||||
|
// entry costs nothing until someone reads the map. Asserted so a later
|
||||||
|
// tidy-up of "unreachable entries" has to meet the ruling first.
|
||||||
|
if _, ok := LookupMIME("audio/webm"); !ok {
|
||||||
|
t.Error("audio/webm was removed; BUG-2963 F6 ruled it stays, unreachable, with its comment")
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// TestTextFamilyStillStoresAsPlain records what this PR does NOT fix, so the
|
||||||
|
// boundary is a test rather than a sentence someone has to find. These types
|
||||||
|
// remain on the allowlist and remain unreachable; making them reachable needs
|
||||||
|
// extension trust, which is a separate change under its own ruling.
|
||||||
|
func TestTextFamilyStillStoresAsPlain(t *testing.T) {
|
||||||
|
for _, tc := range []struct{ body, name string }{
|
||||||
|
{"alert(1);\n", "p.js"},
|
||||||
|
{"answer: 42\n", "p.yaml"},
|
||||||
|
{"# heading\n", "p.md"},
|
||||||
|
} {
|
||||||
|
entry, _, err := ValidateUpload([]byte(tc.body), tc.name)
|
||||||
|
if err != nil {
|
||||||
|
t.Fatalf("%s rejected: %v", tc.name, err)
|
||||||
|
}
|
||||||
|
if entry.MIME != "text/plain" {
|
||||||
|
t.Errorf("%s stored as %q, want text/plain — if this changed, the F5 "+
|
||||||
|
"extension-trust work landed and this test should move with it", tc.name, entry.MIME)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// TestTarAndELFAreNotDistinguishableHere records a LIMITATION as a test,
|
||||||
|
// because it is the kind that otherwise gets rediscovered as a bug.
|
||||||
|
//
|
||||||
|
// The fixture is an ELF header carrying a well-formed tar header in its
|
||||||
|
// padding — `ustar` at 257, valid octal mode/uid/gid/size/mtime, and a
|
||||||
|
// correctly computed checksum. It is ACCEPTED as application/x-tar.
|
||||||
|
//
|
||||||
|
// Production no longer consults archive/tar at all; recognition is by magic,
|
||||||
|
// so this fixture would be accepted on its `ustar` bytes alone. What the
|
||||||
|
// checksum is still FOR is the history: when this package did verify it, and
|
||||||
|
// when archive/tar's own Reader.Next was asked, BOTH accepted this file. That
|
||||||
|
// is why the validation is gone, and it is why the limitation below is a
|
||||||
|
// property of the format rather than a gap someone should close.
|
||||||
|
//
|
||||||
|
// So this asserts the current, understood behaviour rather than a wish. If it
|
||||||
|
// ever starts failing, someone has found a discriminator that the Go standard
|
||||||
|
// library does not have — which is interesting and should be read, not
|
||||||
|
// silently accommodated.
|
||||||
|
func TestTarAndELFAreNotDistinguishableHere(t *testing.T) {
|
||||||
|
b := readFixture(t, "elf-with-valid-tar-checksum.head512")
|
||||||
|
if string(b[1:4]) != "ELF" {
|
||||||
|
t.Fatal("premise failed: the fixture must still be an ELF header")
|
||||||
|
}
|
||||||
|
entry, _, err := ValidateUpload(b, "p.bin")
|
||||||
|
if err != nil {
|
||||||
|
t.Fatalf("refused (%v) — if this is a deliberate improvement, replace this test "+
|
||||||
|
"and say what discriminates the two", err)
|
||||||
|
}
|
||||||
|
if entry.MIME != "application/x-tar" {
|
||||||
|
t.Errorf("stored as %q, want application/x-tar", entry.MIME)
|
||||||
|
}
|
||||||
|
// The property that makes the above tolerable is where the bytes GO, not
|
||||||
|
// what they are called: an archive is never served inline.
|
||||||
|
if entry.ServeInline() {
|
||||||
|
t.Error("application/x-tar is inline-safe; it must be served as an attachment")
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// TestMagicOnlyRecognisesWhatValidationUsedToRefuse is the WIDENING, asserted
|
||||||
|
// rather than described. Every input here was refused by the structural
|
||||||
|
// validation that stood between rounds 1 and 3, and is accepted now.
|
||||||
|
//
|
||||||
|
// The ruling this implements rests on a fact about this door rather than on a
|
||||||
|
// judgement: http.DetectContentType recognises audio/mpeg from the three bytes
|
||||||
|
// "ID3", audio/mpeg is on the allowlist, and this door already serves it
|
||||||
|
// inline. Every signature below is at least as wide as that, so recognising
|
||||||
|
// them this way is the door's existing standard.
|
||||||
|
//
|
||||||
|
// What the second half of each case asserts is the property that makes it
|
||||||
|
// tolerable — where the bytes GO. Recognising a file wrongly moves it between
|
||||||
|
// reviewed allowlist entries; it must never move it into a serving bucket the
|
||||||
|
// entry does not already permit.
|
||||||
|
func TestMagicOnlyRecognisesWhatValidationUsedToRefuse(t *testing.T) {
|
||||||
|
cases := []struct {
|
||||||
|
name string
|
||||||
|
body []byte
|
||||||
|
as string
|
||||||
|
want string
|
||||||
|
wantInline bool
|
||||||
|
}{
|
||||||
|
{"ELF carrying ustar at offset 257", readFixture(t, "elf-with-ustar-magic.head512"),
|
||||||
|
"p.tar", "application/x-tar", false},
|
||||||
|
{"7z signature and nothing else", []byte{0x37, 0x7A, 0xBC, 0xAF, 0x27, 0x1C},
|
||||||
|
"p.7z", "application/x-7z-compressed", false},
|
||||||
|
{"fLaC with no STREAMINFO", []byte("fLaC\x00"),
|
||||||
|
"p.flac", "audio/flac", true},
|
||||||
|
{"BZh9 with nothing after it", []byte("BZh9\x00"),
|
||||||
|
"p.bz2", "application/x-bzip2", false},
|
||||||
|
}
|
||||||
|
for _, tc := range cases {
|
||||||
|
t.Run(tc.name, func(t *testing.T) {
|
||||||
|
entry, code, err := ValidateUpload(tc.body, tc.as)
|
||||||
|
if err != nil {
|
||||||
|
t.Fatalf("refused (code=%s): magic-only recognition should accept this", code)
|
||||||
|
}
|
||||||
|
if entry.MIME != tc.want {
|
||||||
|
t.Errorf("stored as %q, want %q", entry.MIME, tc.want)
|
||||||
|
}
|
||||||
|
// The bucket is the load-bearing half. Archives download; audio
|
||||||
|
// plays inline exactly as audio/mpeg already does.
|
||||||
|
if got := entry.ServeInline(); got != tc.wantInline {
|
||||||
|
t.Errorf("ServeInline() = %v, want %v — recognition may move a file "+
|
||||||
|
"between reviewed types, never into a bucket its entry forbids",
|
||||||
|
got, tc.wantInline)
|
||||||
|
}
|
||||||
|
})
|
||||||
|
}
|
||||||
|
|
||||||
|
// The premise the whole ruling rests on, asserted so it cannot rot: the
|
||||||
|
// stdlib really does recognise audio/mpeg from three bytes, and this door
|
||||||
|
// really does serve it inline.
|
||||||
|
// Exactly three bytes, because three is the claim. A longer input would
|
||||||
|
// pass even if the stdlib needed seven.
|
||||||
|
entry, _, err := ValidateUpload([]byte("ID3"), "p.mp3")
|
||||||
|
if err != nil {
|
||||||
|
t.Fatalf("premise failed: an ID3 header was refused: %v", err)
|
||||||
|
}
|
||||||
|
if entry.MIME != "audio/mpeg" || !entry.ServeInline() {
|
||||||
|
t.Errorf("premise failed: ID3 gave %q inline=%v, want audio/mpeg inline=true — "+
|
||||||
|
"magic-only recognition is justified BY this being the existing standard",
|
||||||
|
entry.MIME, entry.ServeInline())
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// TestRealFilesValidationUsedToRefuse is the direction that made the ruling
|
||||||
|
// necessary. Each is a file a user legitimately has, refused by the structural
|
||||||
|
// validation and accepted now. Refusing these is the defect BUG-2963 exists to
|
||||||
|
// fix, reintroduced by its own fix.
|
||||||
|
func TestRealFilesValidationUsedToRefuse(t *testing.T) {
|
||||||
|
for _, tc := range []struct {
|
||||||
|
fixture string
|
||||||
|
as string
|
||||||
|
want string
|
||||||
|
why string
|
||||||
|
}{
|
||||||
|
{"tar-pax.head512", "archive.tar", "application/x-tar",
|
||||||
|
"a PAX archive leads with a metadata header, so a parse needs more blocks than this door reads"},
|
||||||
|
{"tar-gnu-longname.head512", "archive.tar", "application/x-tar",
|
||||||
|
"a GNU archive whose first filename exceeds 100 bytes leads with a long-name header, likewise"},
|
||||||
|
{"flac-zero-sample-rate.head512", "track.flac", "audio/flac",
|
||||||
|
"RFC 9639 permits a zero sample rate for non-audio samples and still registers it as audio/flac"},
|
||||||
|
} {
|
||||||
|
t.Run(tc.fixture, func(t *testing.T) {
|
||||||
|
entry, code, err := ValidateUpload(readFixture(t, tc.fixture), tc.as)
|
||||||
|
if err != nil {
|
||||||
|
t.Fatalf("refused (code=%s) — %s", code, tc.why)
|
||||||
|
}
|
||||||
|
if entry.MIME != tc.want {
|
||||||
|
t.Errorf("stored as %q, want %q", entry.MIME, tc.want)
|
||||||
|
}
|
||||||
|
})
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// TestMagicStillHasToBeThere keeps the recognisers honest in the only way that
|
||||||
|
// still applies: the magic must be present, in the right place, in full.
|
||||||
|
func TestMagicStillHasToBeThere(t *testing.T) {
|
||||||
|
pad := func(b []byte) []byte {
|
||||||
|
out := make([]byte, 512)
|
||||||
|
copy(out, b)
|
||||||
|
return out
|
||||||
|
}
|
||||||
|
cases := []struct {
|
||||||
|
name string
|
||||||
|
in []byte
|
||||||
|
why string
|
||||||
|
}{
|
||||||
|
{"BZh without the block-size digit", pad([]byte("BZhX")), "the digit is part of the signature"},
|
||||||
|
{"BZh0, below the range", pad([]byte("BZh0")), "bzip2 block sizes are 1..9"},
|
||||||
|
{"BZh:, just above the range", pad([]byte("BZh:")), "':' is '9'+1; the upper bound is a range, not a value"},
|
||||||
|
{"BZ without the h, digit otherwise valid", pad(append([]byte("BZ9"), '1')),
|
||||||
|
"all three letters are the signature — a case that fails the DIGIT check too " +
|
||||||
|
"would leave the prefix check untested"},
|
||||||
|
{"ustar at 256 rather than 257", func() []byte {
|
||||||
|
b := make([]byte, 512)
|
||||||
|
copy(b[256:], []byte("ustar"))
|
||||||
|
return b
|
||||||
|
}(), "the tar magic is at a fixed offset"},
|
||||||
|
{"fLaC not at the start", pad([]byte("\x00\x00fLaC")), "the marker opens the stream"},
|
||||||
|
{"fLa — three of the four marker bytes", pad([]byte("fLa\x00")), "the marker is four bytes, not three"},
|
||||||
|
{"fLac, wrong case on the last byte", pad([]byte("fLac")), "the marker is case-sensitive"},
|
||||||
|
{"five of the six 7z signature bytes", pad([]byte{0x37, 0x7A, 0xBC, 0xAF, 0x27}),
|
||||||
|
"a truncated signature is not the signature"},
|
||||||
|
{"a tar too short to hold the magic offset", make([]byte, 261),
|
||||||
|
"261 bytes cannot contain a magic that ends at 262"},
|
||||||
|
}
|
||||||
|
for _, tc := range cases {
|
||||||
|
t.Run(tc.name, func(t *testing.T) {
|
||||||
|
if got := sniffOpaqueMagic(tc.in); got != "" {
|
||||||
|
t.Errorf("sniffOpaqueMagic = %q, want no match — %s", got, tc.why)
|
||||||
|
}
|
||||||
|
})
|
||||||
|
}
|
||||||
|
|
||||||
|
// Lengths each recogniser indexes past, walked so a bounds error shows up
|
||||||
|
// as a failure rather than as a panic in production.
|
||||||
|
for _, n := range []int{0, 1, 2, 3, 4, 5, 6, 7, 8, 260, 261, 262, 511, 512} {
|
||||||
|
sniffOpaqueMagic(make([]byte, n))
|
||||||
|
}
|
||||||
|
if got := sniffOpaqueMagic(nil); got != "" {
|
||||||
|
t.Errorf("sniffOpaqueMagic(nil) = %q, want no match", got)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// TestEBMLDocTypeSurvivesPaddingPastTheHead covers a round-3 finding that the
|
||||||
|
// magic-only ruling does not touch, because the DocType parse stays: a header
|
||||||
|
// declaring a DocType payload longer than the bytes we hold is still
|
||||||
|
// answerable when the VALUE is complete in hand, since it ends at its first
|
||||||
|
// NUL.
|
||||||
|
func TestEBMLDocTypeSurvivesPaddingPastTheHead(t *testing.T) {
|
||||||
|
// EBML header declaring a 600-byte DocType payload: "matroska", a NUL,
|
||||||
|
// and padding that runs past anything this door reads.
|
||||||
|
body := append([]byte("matroska\x00"), make([]byte, 591)...)
|
||||||
|
head := []byte{0x1A, 0x45, 0xDF, 0xA3, 0x42, 0x5C, 0x42, 0x82, 0x42, 0x58}
|
||||||
|
head = append(head, body...)
|
||||||
|
if len(head) > 512 {
|
||||||
|
head = head[:512]
|
||||||
|
}
|
||||||
|
if got := sniffEBMLDocType(head); got != "video/x-matroska" {
|
||||||
|
t.Errorf("sniffEBMLDocType = %q, want video/x-matroska — the value is complete "+
|
||||||
|
"in the bytes we hold even though its declared padding is not", got)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// TestEBMLParseDetails covers the two DocType-walk properties that survive the
|
||||||
|
// magic-only ruling untouched. They were lost when the structural-validation
|
||||||
|
// tests were cut wholesale, and a mutation run caught the gap: both mutations
|
||||||
|
// below had gone from detected to surviving.
|
||||||
|
func TestEBMLParseDetails(t *testing.T) {
|
||||||
|
t.Run("DocType ends at its first NUL", func(t *testing.T) {
|
||||||
|
// A real Matroska whose DocType payload is "matroska\x00junk" with a
|
||||||
|
// declared length of 13. Bytes after the terminator are padding;
|
||||||
|
// trimming instead of terminating stored this as WebM.
|
||||||
|
if got := SniffMIME(readFixture(t, "matroska-nul-terminated-doctype.head512")); got != "video/x-matroska" {
|
||||||
|
t.Errorf("SniffMIME = %q, want video/x-matroska", got)
|
||||||
|
}
|
||||||
|
})
|
||||||
|
|
||||||
|
t.Run("reserved all-ones EBML IDs are refused", func(t *testing.T) {
|
||||||
|
// 0xFF is a reserved ID, not a valid element. Accepting it let junk
|
||||||
|
// act as a zero-length child and carry the walk onward to a DocType
|
||||||
|
// that followed it.
|
||||||
|
bad := []byte{0x1A, 0x45, 0xDF, 0xA3, 0x8D, 0xFF, 0x80,
|
||||||
|
0x42, 0x82, 0x88, 'm', 'a', 't', 'r', 'o', 's', 'k', 'a'}
|
||||||
|
if got := sniffEBMLDocType(bad); got != "" {
|
||||||
|
t.Errorf("sniffEBMLDocType = %q, want no answer — the walk crossed a reserved ID", got)
|
||||||
|
}
|
||||||
|
// Control: the same shape WITHOUT the reserved ID must parse, so the
|
||||||
|
// leg above fails on the ID rather than on anything else about it.
|
||||||
|
ok := []byte{0x1A, 0x45, 0xDF, 0xA3, 0x8B,
|
||||||
|
0x42, 0x82, 0x88, 'm', 'a', 't', 'r', 'o', 's', 'k', 'a'}
|
||||||
|
if got := sniffEBMLDocType(ok); got != "video/x-matroska" {
|
||||||
|
t.Errorf("premise failed: control sniffed %q, want video/x-matroska", got)
|
||||||
|
}
|
||||||
|
})
|
||||||
|
}
|
||||||
|
|
||||||
|
// TestCollidingMagicIsArbitratedByExtension covers the collision both ways.
|
||||||
|
// More than one magic can match the same bytes, and review produced ordinary
|
||||||
|
// files on each side, so any fixed winner refuses somebody:
|
||||||
|
//
|
||||||
|
// - a tar whose first member is named "fLaC.txt" carries the FLAC marker at
|
||||||
|
// offset zero, because a tar header opens with a filename;
|
||||||
|
// - a FLAC whose Vorbis COMMENT tag contains "ustar" carries the tar magic
|
||||||
|
// at offset 257, because those tags are arbitrary UTF-8 (RFC 9639 §8.6).
|
||||||
|
//
|
||||||
|
// The extension arbitrates. It cannot introduce a type — both candidates are
|
||||||
|
// ones the bytes matched — it only says which reading the uploader meant.
|
||||||
|
func TestCollidingMagicIsArbitratedByExtension(t *testing.T) {
|
||||||
|
tarFile := readFixture(t, "tar-flac-named-member.head512")
|
||||||
|
flacFile := readFixture(t, "flac-ustar-in-comment.head512")
|
||||||
|
|
||||||
|
// Premise: both files really are ambiguous, or this arbitrates nothing.
|
||||||
|
for _, tc := range []struct {
|
||||||
|
name string
|
||||||
|
body []byte
|
||||||
|
}{{"tar with a fLaC-named member", tarFile}, {"flac with ustar in a comment", flacFile}} {
|
||||||
|
got := sniffOpaqueCandidates(tc.body)
|
||||||
|
if len(got) < 2 {
|
||||||
|
t.Fatalf("premise failed: %s matched %v, want at least two candidates", tc.name, got)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
for _, tc := range []struct {
|
||||||
|
name string
|
||||||
|
body []byte
|
||||||
|
as string
|
||||||
|
want string
|
||||||
|
}{
|
||||||
|
{"tar named .tar", tarFile, "archive.tar", "application/x-tar"},
|
||||||
|
{"flac named .flac", flacFile, "track.flac", "audio/flac"},
|
||||||
|
} {
|
||||||
|
t.Run(tc.name, func(t *testing.T) {
|
||||||
|
entry, code, err := ValidateUpload(tc.body, tc.as)
|
||||||
|
if err != nil {
|
||||||
|
t.Fatalf("refused (code=%s); the extension names a candidate the bytes matched", code)
|
||||||
|
}
|
||||||
|
if entry.MIME != tc.want {
|
||||||
|
t.Errorf("stored as %q, want %q", entry.MIME, tc.want)
|
||||||
|
}
|
||||||
|
})
|
||||||
|
}
|
||||||
|
|
||||||
|
// When the extension names neither candidate, the DEFAULT order decides,
|
||||||
|
// and that is the only case where it does anything — arbitration settles
|
||||||
|
// both collisions above whichever way the list is ordered. Tar leads
|
||||||
|
// because its magic is at a fixed offset rather than at a prefix, so it is
|
||||||
|
// the candidate least likely to be an accident of another format's leading
|
||||||
|
// bytes. A mutation run is what showed this needed asserting: reordering
|
||||||
|
// the list changed no other test.
|
||||||
|
if entry, _, err := ValidateUpload(tarFile, "data.bin"); err != nil {
|
||||||
|
t.Errorf("colliding bytes with an unmapped extension were refused: %v", err)
|
||||||
|
} else if entry.MIME != "application/x-tar" {
|
||||||
|
t.Errorf("stored as %q, want application/x-tar — with no extension to arbitrate, "+
|
||||||
|
"the documented default order stands", entry.MIME)
|
||||||
|
}
|
||||||
|
|
||||||
|
// An extension naming NEITHER candidate must not invent a third reading.
|
||||||
|
// The property is about the TYPE, not about acceptance: .zip names a type
|
||||||
|
// whose magic is absent here, so the default candidate stands. That the
|
||||||
|
// upload then succeeds is the ordinary category rule — tar and zip are
|
||||||
|
// both archives — and is not something arbitration decided.
|
||||||
|
entry, _, err := ValidateUpload(flacFile, "mystery.zip")
|
||||||
|
if err == nil && entry.MIME == "application/zip" {
|
||||||
|
t.Error("a .zip name made colliding bytes into a zip; the extension may choose " +
|
||||||
|
"among candidates the bytes matched, never add one")
|
||||||
|
}
|
||||||
|
if err == nil && entry.MIME != "application/x-tar" {
|
||||||
|
t.Errorf("stored as %q, want the default candidate application/x-tar", entry.MIME)
|
||||||
|
}
|
||||||
|
}
|
||||||
@@ -89,10 +89,21 @@ func TestServeInline(t *testing.T) {
|
|||||||
t.Errorf("ServeInline(%q) = false, want true (inline-safe)", m)
|
t.Errorf("ServeInline(%q) = false, want true (inline-safe)", m)
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
// application/xml, text/yaml and application/javascript were in this list
|
||||||
|
// until BUG-2963 F6 removed them from the allowlist as unreachable
|
||||||
|
// spellings. `must` fails on a type the allowlist refuses, so leaving them
|
||||||
|
// here would assert about entries that no longer exist.
|
||||||
|
//
|
||||||
|
// The substitutes below are the siblings that remain ON THE ALLOWLIST.
|
||||||
|
// That is all they are: text/javascript and application/yaml are not
|
||||||
|
// reachable as stored types either (a .js or .yaml upload is stored as
|
||||||
|
// text/plain today). This test asks what ServeInline answers for an
|
||||||
|
// allowlist entry, which is a question about the entry and not about
|
||||||
|
// whether an upload can wear it.
|
||||||
download := []string{
|
download := []string{
|
||||||
"text/xml", "application/xml", "application/json", "text/csv",
|
"text/xml", "application/json", "text/csv",
|
||||||
"text/markdown", "application/msword", "application/zip",
|
"text/markdown", "application/msword", "application/zip",
|
||||||
"text/html", "text/javascript", "application/javascript",
|
"text/html", "text/javascript", "application/yaml",
|
||||||
}
|
}
|
||||||
for _, m := range download {
|
for _, m := range download {
|
||||||
if must(m).ServeInline() {
|
if must(m).ServeInline() {
|
||||||
|
|||||||
+79
@@ -23,3 +23,82 @@ Two gaps, recorded rather than papered over:
|
|||||||
- **No file whose MAJOR brand is `mif1` from Apple.** `sips -s format heif`
|
- **No file whose MAJOR brand is `mif1` from Apple.** `sips -s format heif`
|
||||||
writes no file on that machine. The libheif `still-mif1.heif` covers the
|
writes no file on that machine. The libheif `still-mif1.heif` covers the
|
||||||
major-`mif1` shape from a different encoder instead of a guessed one.
|
major-`mif1` shape from a different encoder instead of a guessed one.
|
||||||
|
|
||||||
|
## BUG-2963 fixtures — formats the sniffer learned to recognise
|
||||||
|
|
||||||
|
Same rule as above: real encoder output, truncated to the 512 bytes
|
||||||
|
`SniffMIME` actually reads. `bzip2.head512` and `sevenzip.head512` are shorter
|
||||||
|
than 512 because the whole file is.
|
||||||
|
|
||||||
|
| file | produced by | what it proves |
|
||||||
|
|---|---|---|
|
||||||
|
| `tar.head512` | GNU `tar -cf` (tar 1.34, Linux) | `ustar` at offset 257 — the magic is not at the start, which is why no prefix matcher finds it |
|
||||||
|
| `bzip2.head512` | `bzip2 -c` (1.0.8), complete file (88 B) | `BZh` plus the block-size digit |
|
||||||
|
| `sevenzip.head512` | `py7zr` 1.1.3, complete file (192 B) | the six-byte 7z signature |
|
||||||
|
| `flac.head512` | libsndfile 1.2.2 via `soundfile`, FLAC format | the `fLaC` stream marker |
|
||||||
|
| `aac-adts.head512` | FFmpeg 7.1 `-c:a aac -f adts` | an ADTS sync (`ff f1`) — the one signature weak enough to need its extension |
|
||||||
|
| `matroska.head512` | FFmpeg 7.1 `-f matroska` | EBML magic with DocType `matroska` at offset 24 |
|
||||||
|
| `webm.head512` | FFmpeg 7.1 `-f webm` | EBML magic with DocType `webm` at offset 24 — the control that stops the DocType read from answering Matroska for everything |
|
||||||
|
| `avi.head512` | FFmpeg 7.1 `-f avi` | RIFF/AVI, which the stdlib names `video/avi` against the allowlist's `video/x-msvideo` |
|
||||||
|
|
||||||
|
### Fixtures for files the recogniser must ACCEPT
|
||||||
|
|
||||||
|
Real files that three rounds of structural validation refused. Each is a file a
|
||||||
|
user legitimately has, and refusing them is the defect BUG-2963 exists to fix —
|
||||||
|
reintroduced, for a while, by its own fix.
|
||||||
|
|
||||||
|
| file | produced by | what it proves |
|
||||||
|
|---|---|---|
|
||||||
|
| `tar-pax.head512` | `tar --format=pax -cf` (tar 1.34) | a PAX archive leads with a metadata header, so a full parse needs more blocks than this door ever reads |
|
||||||
|
| `tar-gnu-longname.head512` | `tar --format=gnu -cf` with a 144-character member name | same shape, via GNU's long-name header |
|
||||||
|
| `flac-zero-sample-rate.head512` | `flac.head512` with the sample-rate field zeroed | RFC 9639 permits a zero sample rate for non-audio samples and still registers the result as `audio/flac` |
|
||||||
|
|
||||||
|
### Adversarial fixtures — the round-1 findings, kept as tests
|
||||||
|
|
||||||
|
These exist to be REFUSED or to be typed correctly against a check that used
|
||||||
|
to get them wrong. Each was produced by an adversarial review round that broke
|
||||||
|
the first version of these signatures.
|
||||||
|
|
||||||
|
| file | produced by | what it proves |
|
||||||
|
|---|---|---|
|
||||||
|
| `tar-bmp-firstmember.head512` | GNU `tar -cf` on a file named `BM.txt` | a structurally valid tar the stdlib reads as `image/bmp` — a real file exercising competing detections, refused before this change and still refused |
|
||||||
|
| `elf-with-ustar-magic.head512` | hand-built | an ELF header with `ustar` at offset 257. It is ACCEPTED now — recognition is by magic — and the test says so. It was written when it was refused, and is kept because the finding it comes from used a real, executing binary |
|
||||||
|
| `webm-void-says-matroska.head512` | FFmpeg WebM with a Void element containing the string `matroska`, header size widened to match | a substring search calls this Matroska; its DocType is `webm` |
|
||||||
|
| `matroska-void-padded.head512` | FFmpeg Matroska with 40 bytes of Void padding, header size widened to match | DocType moves to offset 66, past any fixed leading window — the mistyping this change fixes, still live under a search |
|
||||||
|
| `elf-with-valid-tar-checksum.head512` | hand-built | an ELF header carrying a well-formed tar header in its padding, checksum included. ACCEPTED, deliberately — and when this package still verified checksums, `archive/tar`'s own reader accepted it too. It records a limitation rather than a defect |
|
||||||
|
| `tar-flac-named-member.head512` | GNU `tar -cf` on a file named `fLaC.txt` | an ordinary archive carrying another format's prefix magic in its member NAME |
|
||||||
|
| `flac-ustar-in-comment.head512` | a real FLAC with a Vorbis COMMENT block whose vendor string places `ustar` at offset 257; decodes under libsndfile | the same collision from the other side — Vorbis tags are arbitrary UTF-8, so audio can carry the tar magic as easily as a tar can carry an audio marker |
|
||||||
|
| `matroska-nul-terminated-doctype.head512` | `sample.mkv` with its DocType payload rewritten to `matroska\0junk`, declared length 13 | a DocType value ends at its first NUL; trimming instead stored a real Matroska as WebM |
|
||||||
|
|
||||||
|
Void elements are legal anywhere in an EBML header and their contents are
|
||||||
|
meaningless by specification, which is why only a parse can tell payload from
|
||||||
|
padding.
|
||||||
|
|
||||||
|
The two EBML entries say "ffprobe-readable" of the FILES THEY WERE MADE FROM.
|
||||||
|
What is committed is the first 512 bytes, as with every fixture here, so
|
||||||
|
running ffprobe on the committed file fails with a premature EOF — that is the
|
||||||
|
truncation, not the file.
|
||||||
|
|
||||||
|
### Ogg fixtures — kept for a format the sniffer does NOT recognise
|
||||||
|
|
||||||
|
Both are refused, and both are kept so the next person to reach for an
|
||||||
|
`application/ogg` alias meets the evidence before writing one.
|
||||||
|
|
||||||
|
| file | produced by | what it proves |
|
||||||
|
|---|---|---|
|
||||||
|
| `ogg-opus.head512` | FFmpeg 7.1 `-c:a libopus -f ogg` | ordinary Ogg audio, refused — `audio/ogg` stays unreachable |
|
||||||
|
| `ogg-vp8-video.head512` | FFmpeg 7.1 `-c:v libvpx -f ogg` | a real Ogg file whose first packet is `OVP80`: video in an Ogg container, which an unconditional alias accepted as inline audio |
|
||||||
|
|
||||||
|
An alias was written, ruled in, and removed: whether an Ogg container is
|
||||||
|
audio-only cannot be decided from its first page, because Ogg multiplexes and a
|
||||||
|
second video stream's pages come later than any 512-byte sniff can see.
|
||||||
|
|
||||||
|
The FFmpeg used is the one bundled with Remotion
|
||||||
|
(`@remotion/compositor-linux-x64-gnu`), transcoded from real project assets;
|
||||||
|
there is no system FFmpeg on the machine these were made on.
|
||||||
|
|
||||||
|
**Gap, recorded rather than papered over:** the media fixtures come from one
|
||||||
|
FFmpeg build, apart from `flac-ustar-in-comment.head512`, which is a FLAC
|
||||||
|
assembled by hand from a real one and verified decodable by libsndfile. A second encoder would be worth having for the EBML pair in
|
||||||
|
particular, since the DocType read is the only check here that depends on where
|
||||||
|
a muxer places a string rather than on a fixed prefix.
|
||||||
|
|||||||
BIN
Binary file not shown.
BIN
Binary file not shown.
BIN
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
BIN
Binary file not shown.
Binary file not shown.
Binary file not shown.
BIN
Binary file not shown.
BIN
Binary file not shown.
Binary file not shown.
BIN
Binary file not shown.
Binary file not shown.
|
After Width: | Height: | Size: 512 B |
Binary file not shown.
Binary file not shown.
BIN
Binary file not shown.
BIN
Binary file not shown.
Binary file not shown.
BIN
Binary file not shown.
@@ -413,6 +413,13 @@ func mimePredicateForCategory(category string) (frag string, args []any, ok bool
|
|||||||
"application/rtf",
|
"application/rtf",
|
||||||
})
|
})
|
||||||
case "text":
|
case "text":
|
||||||
|
// Wider than the upload allowlist on purpose. This filters ROWS THAT
|
||||||
|
// EXIST by display category; the allowlist decides what may be
|
||||||
|
// created. application/xml, text/yaml and application/javascript left
|
||||||
|
// the allowlist in BUG-2963 F6 as spellings nothing could produce, and
|
||||||
|
// they are kept here because a filter that stops matching a type costs
|
||||||
|
// something (a row nobody can find) while one that matches a type no
|
||||||
|
// row carries costs nothing.
|
||||||
return mimeInPredicate([]string{
|
return mimeInPredicate([]string{
|
||||||
"text/plain", "text/markdown", "text/csv", "text/tab-separated-values",
|
"text/plain", "text/markdown", "text/csv", "text/tab-separated-values",
|
||||||
"application/json", "application/xml", "text/xml",
|
"application/json", "application/xml", "text/xml",
|
||||||
|
|||||||
@@ -397,11 +397,16 @@ export function canBrowserPreview(mime: string | null | undefined): boolean {
|
|||||||
* served as an attachment.
|
* served as an attachment.
|
||||||
*
|
*
|
||||||
* MIME-EXACT, NEVER BY CATEGORY. `CategoryText` server-side CONTAINS the
|
* MIME-EXACT, NEVER BY CATEGORY. `CategoryText` server-side CONTAINS the
|
||||||
* force-download bucket — `text/html`, `text/javascript` and
|
* force-download bucket — `text/html` and `text/javascript` are both
|
||||||
* `application/javascript` are all `CategoryText` (mime.go). A
|
* `CategoryText` (mime.go). A category test would therefore admit exactly
|
||||||
* category test would therefore admit exactly the types PLAN-2393 DR-6
|
* the types PLAN-2393 DR-6 forbids inlining. An allowlist excludes them by
|
||||||
* forbids inlining. An allowlist excludes them by construction rather
|
* construction rather than by anyone remembering to.
|
||||||
* than by anyone remembering to.
|
*
|
||||||
|
* `application/javascript` used to be named here as a third example. It left
|
||||||
|
* the server allowlist in BUG-2963 F6 as an unreachable spelling, so the
|
||||||
|
* server no longer has a category for it at all — but it stays excluded from
|
||||||
|
* the allowlists below, because the string can still reach this client from
|
||||||
|
* somewhere that is not our upload door.
|
||||||
*
|
*
|
||||||
* SMALLER THAN WHAT WE COULD RENDER, on purpose. `text/csv`,
|
* SMALLER THAN WHAT WE COULD RENDER, on purpose. `text/csv`,
|
||||||
* `text/tab-separated-values`, JSON, XML, YAML and TOML are all
|
* `text/tab-separated-values`, JSON, XML, YAML and TOML are all
|
||||||
|
|||||||
@@ -63,11 +63,8 @@
|
|||||||
"text/html": "text",
|
"text/html": "text",
|
||||||
"text/xml": "text",
|
"text/xml": "text",
|
||||||
"text/javascript": "text",
|
"text/javascript": "text",
|
||||||
"text/yaml": "text",
|
|
||||||
"application/json": "text",
|
"application/json": "text",
|
||||||
"application/xml": "text",
|
|
||||||
"application/yaml": "text",
|
"application/yaml": "text",
|
||||||
"application/toml": "text",
|
"application/toml": "text"
|
||||||
"application/javascript": "text"
|
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -45,9 +45,11 @@ export type SurfaceRendererId = 'raster-image' | 'text';
|
|||||||
* The renderer for a MIME, or `null` when none claims it (→ the icon fallback).
|
* The renderer for a MIME, or `null` when none claims it (→ the icon fallback).
|
||||||
* `'raster-image'` is exactly the DR-16 raster allowlist and `'text'` exactly the
|
* `'raster-image'` is exactly the DR-16 raster allowlist and `'text'` exactly the
|
||||||
* `canPreviewAsText` allowlist; unsafe, unknown and unresolved (null) MIMEs all
|
* `canPreviewAsText` allowlist; unsafe, unknown and unresolved (null) MIMEs all
|
||||||
* return `null`. In particular the force-download bucket (`text/html`,
|
* return `null`. In particular the force-download bucket (`text/html` and
|
||||||
* `text/javascript`, `application/javascript` — all `CategoryText` server-side)
|
* `text/javascript` — both `CategoryText` server-side; `application/javascript`
|
||||||
* claims no renderer, per PLAN-2393 DR-6.
|
* left that allowlist in BUG-2963 F6 but is still excluded here, since the
|
||||||
|
* string can arrive from outside our upload door) claims no renderer, per
|
||||||
|
* PLAN-2393 DR-6.
|
||||||
*/
|
*/
|
||||||
export function getSurfaceRenderer(mime: string | null): SurfaceRendererId | null {
|
export function getSurfaceRenderer(mime: string | null): SurfaceRendererId | null {
|
||||||
if (canOpenInViewer(mime)) return 'raster-image';
|
if (canOpenInViewer(mime)) return 'raster-image';
|
||||||
|
|||||||
@@ -219,8 +219,9 @@
|
|||||||
* TWO ARMS NOW LOAD BYTES, and the invariant is unchanged in kind: the text
|
* TWO ARMS NOW LOAD BYTES, and the invariant is unchanged in kind: the text
|
||||||
* arm's bytes never become active same-origin content either. They are
|
* arm's bytes never become active same-origin content either. They are
|
||||||
* rendered through `sanitizeMarkdownHtml`, and the types for which that would
|
* rendered through `sanitizeMarkdownHtml`, and the types for which that would
|
||||||
* be the wrong bet — `text/html`, `text/javascript`, `application/javascript`,
|
* be the wrong bet — `text/html` and `text/javascript`, which share the
|
||||||
* which share the server's `CategoryText` with markdown — are excluded from
|
* server's `CategoryText` with markdown, plus `application/javascript`,
|
||||||
|
* which the server stopped classifying at all in BUG-2963 F6 — are excluded from
|
||||||
* `canPreviewAsText` by allowlist, so they never reach the arm. Exactly one
|
* `canPreviewAsText` by allowlist, so they never reach the arm. Exactly one
|
||||||
* loader is armed at a time; the load effect disposes the other, so an arm
|
* loader is armed at a time; the load effect disposes the other, so an arm
|
||||||
* flip releases the previous arm's bytes rather than leaving them behind
|
* flip releases the previous arm's bytes rather than leaving them behind
|
||||||
|
|||||||
Reference in New Issue
Block a user