From eccc6cf12f7897908db90e70a33c0e8a89d5ba6d Mon Sep 17 00:00:00 2001 From: David Mzareulyan Date: Mon, 17 Aug 2020 15:50:13 +0300 Subject: [PATCH] Restrict the allowed attachment types to explicit list --- .../api/v1/AttachmentsController.js | 5 ++ app/models/attachment.js | 63 ++++++--------- config/default.js | 16 +++- test/fixtures/lol.html | 5 ++ test/fixtures/test-image.900x300.webp | Bin 0 -> 1210 bytes test/integration/models/attachment2.js | 73 ++++++++++++++++++ 6 files changed, 123 insertions(+), 39 deletions(-) create mode 100644 test/fixtures/lol.html create mode 100644 test/fixtures/test-image.900x300.webp create mode 100644 test/integration/models/attachment2.js diff --git a/app/controllers/api/v1/AttachmentsController.js b/app/controllers/api/v1/AttachmentsController.js index 63439e82a..37736c80b 100644 --- a/app/controllers/api/v1/AttachmentsController.js +++ b/app/controllers/api/v1/AttachmentsController.js @@ -4,6 +4,7 @@ import createDebug from 'debug'; import { reportError } from '../../../support/exceptions'; import { serializeAttachment } from '../../../serializers/v2/post'; import { serializeUsersByIds } from '../../../serializers/v2/user'; +import { UnsupportedTypeError } from '../../../models/attachment'; export default class AttachmentsController { @@ -48,6 +49,10 @@ export default class AttachmentsController { return; } + if (e instanceof UnsupportedTypeError) { + e.status = 415; + } + reportError(ctx)(e); } }) diff --git a/app/models/attachment.js b/app/models/attachment.js index 6d9a264f6..9b9502191 100644 --- a/app/models/attachment.js +++ b/app/models/attachment.js @@ -1,5 +1,6 @@ import { promises as fs, createReadStream } from 'fs'; import { execFile } from 'child_process'; +import { extname } from 'path'; import config from 'config'; import { promisify, promisifyAll } from 'bluebird'; @@ -64,6 +65,8 @@ async function mimeTypeDetect(fileName, filePath) { const execFileAsync = promisify(execFile); +export class UnsupportedTypeError extends Error {} + export function addModel(dbAdapter) { return class Attachment { constructor(params) { @@ -136,16 +139,11 @@ export function addModel(dbAdapter) { this.mimeType = this.file.type; // Determine initial file extension - // (it might be overridden later when we know MIME type from its contents) - // TODO: extract to config - const supportedExtensions = /\.(jpe?g|png|gif|mp3|m4a|ogg|wav|txt|pdf|docx?|pptx?|xlsx?)$/i; - - if (this.fileName && this.fileName.match(supportedExtensions) !== null) { - this.fileExtension = this.fileName - .match(supportedExtensions)[1] - .toLowerCase(); - } else { - this.fileExtension = ''; + // (it will be overridden later when we know MIME type from its contents) + this.fileExtension = extname(this.fileName || '').toLowerCase(); + + if (this.fileExtension.startsWith('.')) { + this.fileExtension = this.fileExtension.substring(1); } await this.handleMedia(); @@ -251,39 +249,26 @@ export function addModel(dbAdapter) { const tmpAttachmentFile = this.file.path; const tmpAttachmentFileName = this.file.name; - const supportedImageTypes = { - 'image/jpeg': 'jpg', - 'image/png': 'png', - 'image/gif': 'gif', - 'image/svg+xml': 'svg' - }; - const supportedAudioTypes = { - 'audio/mpeg': 'mp3', - 'audio/x-m4a': 'm4a', - 'audio/m4a': 'm4a', - 'audio/mp4': 'm4a', - 'audio/ogg': 'ogg', - 'audio/x-wav': 'wav' - }; - this.mimeType = await mimeTypeDetect( tmpAttachmentFileName, tmpAttachmentFile ); debug(`Mime-type of ${tmpAttachmentFileName} is ${this.mimeType}`); - if (supportedImageTypes[this.mimeType]) { - // Set media properties for 'image' type + if (this.mimeType.startsWith('image/')) { this.mediaType = 'image'; - this.fileExtension = supportedImageTypes[this.mimeType]; - this.noThumbnail = '1'; // this may be overriden below - await this.handleImage(tmpAttachmentFile); - } else if (supportedAudioTypes[this.mimeType]) { - // Set media properties for 'audio' type + } else if (this.mimeType.startsWith('audio/')) { this.mediaType = 'audio'; - this.fileExtension = supportedAudioTypes[this.mimeType]; - this.noThumbnail = '1'; + } else { + this.mediaType = 'general'; + } + + this.noThumbnail = '1'; // this may be overriden below + if (this.mediaType === 'image') { + await this.handleImage(tmpAttachmentFile); + } else if (this.mediaType === 'audio') { + // Set media properties for 'audio' type if (this.fileExtension === 'm4a') { this.mimeType = 'audio/mp4'; // mime-type compatible with music-metadata } @@ -300,10 +285,12 @@ export function addModel(dbAdapter) { } else { this.artist = metadata.artist; } - } else { - // Set media properties for 'general' type - this.mediaType = 'general'; - this.noThumbnail = '1'; + } + + this.fileExtension = config.attachments.supportedTypes[this.mimeType]; + + if (!this.fileExtension) { + throw new UnsupportedTypeError(`Unsupported MIME type: ${this.mimeType}`); } // Store an original attachment diff --git a/config/default.js b/config/default.js index 79047d3a1..05a513025 100644 --- a/config/default.js +++ b/config/default.js @@ -128,7 +128,21 @@ config.attachments = { path: 'attachments/thumbnails2/', // must have trailing slash bounds: { width: 1050, height: 350 } } - } + }, + // MIME type to file extension map + supportedTypes: { + 'image/jpeg': 'jpg', + 'image/png': 'png', + 'image/gif': 'gif', + 'image/webp': 'webp', + 'audio/mpeg': 'mp3', + 'audio/mp4': 'm4a', + 'video/mp4': 'mp4', + 'video/quicktime': 'mov', + 'application/pdf': 'pdf', + 'application/zip': 'zip', + 'text/plain': 'txt', + }, }; config.profilePictures = { defaultProfilePictureMediumUrl: 'http://placekitten.com/50/50', diff --git a/test/fixtures/lol.html b/test/fixtures/lol.html new file mode 100644 index 000000000..312de3e33 --- /dev/null +++ b/test/fixtures/lol.html @@ -0,0 +1,5 @@ + + + diff --git a/test/fixtures/test-image.900x300.webp b/test/fixtures/test-image.900x300.webp new file mode 100644 index 0000000000000000000000000000000000000000..643e1999143780397cfd1561d6f89df65c3a69cd GIT binary patch literal 1210 zcmV;r1V#H&Nk&Gp1ONb6MM6+kP&iDb1ONapgTqPyf1svq1BO4{?yocyw~eIq-{&$O ze~&=Yrhn#b|NO`Q@2OY2>QS${m7^?VDN`O$O9`m}0Nw5X(@4%|c#fHA>M(QW7Q(bw zhMAd@ZjbM(r9KR0X8XXxKM-c-q*}(Avz}-DlYX|7LRTrG{}3EWav~+B&tR%FOfU2@ zA226@Q}CbQKf!;3{{;UD{uBHs_)qYk;6K5Cg8u~n33Kq?%~u?0x_Gp}eTxnn7jV|Q znegiQ)DB0QT-A%m@&v%ze{4<#s4ZFyS0r zj1{E55mm0{#a9_CNa&N4q3sGsqdx^)QOu&z>#+8UjX+D2t9tQNx}fb_^GXpGVtU~~ z6zAgof@+-B$F{cvaxg{JJjLcq^< zp00{+yNdm`x)bSG&9I;Wkt$d7;-&0mpW`ERHg4;+k>E_87H$2j6qRQ#9V?iWnFN}u zT+NG@vWC|!mNDUM+-_>_3#P(kIvSM%bftmGGqRSd@44NE?b6YtN^vgBo! zc;j@f$yL4hC@UCuWP$y1b2;T$Ft4%JNcPa@V$3>H5{6=-nq1Y3?{c>>Ri+&u$MdF_ zHj6IyyVxdsc6~MhPsKOAnq1Y3kFuI!$2HXEYFp9X&g-R@HV3aUB8y1wCmUyycgs!S z#Yfr0(-swKS@D!C4AeO*(tpn}=ixlX+Q=>`U`&ptBNa@TJq$WOSRGS24r2v=HyVwy zs2pf=RWClu5X@M0*%+LRn>9`_N=Oj90c!J0xaB7C;-gcMeNh}swe%<(?bpW3 z*$D%Y`FSg^r$-5wa#oZ8IOOHL2Ap+89%j>{1oZ#^eEj&!MHuyQ%nro#^bl=nH&H@- zlmydQm9;JZ-o%)(a!gxuiEIvJ-!Km*^YC$wDNjC-ns;x`uYJB{dU-lS>aezzw6xUc z+h`eie^%8z@eG<{BpF@Yowl0ix17mf%8I%Y&w4Ux=%tpnqVBh>WIUs9wX8BwIbj$A z|Nd6%Wt3B1bJ=O$~dw+rQtiGI@R%EMvcM z_2{3)TaJzty^Fiu51UeH*}AE6UMi-_>zZ8Ei=R@JJ^8hq?7piwEAHiW>rG?$T;yw| z*WM5bo?39%hUTr(;FKm;_2Q>=L4RaEE-0#0j&J@4qrb0$`~OWl`uiks3jP!PC-_hBpWr{ie}ex6 Y{|WvR{3rNN@Sos6!GD7PoMd1C0Iyec@Bjb+ literal 0 HcmV?d00001 diff --git a/test/integration/models/attachment2.js b/test/integration/models/attachment2.js new file mode 100644 index 000000000..e78c9ac16 --- /dev/null +++ b/test/integration/models/attachment2.js @@ -0,0 +1,73 @@ +/* eslint-env node, mocha */ +/* global $pg_database */ +import path from 'path'; +import os from 'os'; +import { promises as fs } from 'fs'; + +import expect from 'unexpected'; + +import cleanDB from '../../dbCleaner'; +import { Attachment, User } from '../../../app/models'; + + +describe('Attachment2', () => { + before(() => cleanDB($pg_database)); + let luna; + before(async () => { + luna = new User({ username: 'luna', password: 'pw' }); + await luna.create(); + }); + + it(`should create JPEG attachment`, async () => { + const file = await uploadFile( + 'test-image-exif-rotated.900x300.jpg', + 'image/jpeg' + ); + const att = new Attachment({ file, userId: luna.id }); + await att.create(); + expect(att, 'to satisfy', { + mediaType: 'image', + mimeType: 'image/jpeg', + fileExtension: 'jpg', + }); + await att.deleteFiles(); + }); + + it(`should not create HTML attachment`, async () => { + const file = await uploadFile('lol.html', 'text/html'); + const att = new Attachment({ file, userId: luna.id }); + await expect(att.create(), 'to be rejected with', 'Unsupported MIME type: text/html'); + await fs.unlink(file.path); + }); + + it(`should create WebP attachment`, async () => { + const file = await uploadFile( + 'test-image.900x300.webp', + 'image/webp' + ); + const att = new Attachment({ file, userId: luna.id }); + await att.create(); + expect(att, 'to satisfy', { + mediaType: 'image', + mimeType: 'image/webp', + fileExtension: 'webp', + }); + await att.deleteFiles(); + }); +}); + +const fixturesDir = path.resolve(__dirname, '../../fixtures'); +const tmpDir = os.tmpdir(); + +async function uploadFile(fileName, mimeType) { + const uplPath = path.join(tmpDir, fileName); + // Upload file + await fs.copyFile(path.join(fixturesDir, fileName), uplPath); + const st = await fs.stat(uplPath); + return { + path: uplPath, + name: fileName, + size: st.size, + type: mimeType, + }; +}