Restrict the allowed attachment types to explicit list

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);
       }
     })

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

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',

test/fixtures/lol.html

@@ -0,0 +1,5 @@
+<html>
+  <script>
+    alert('lol');
+  </script>
+</html>

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,
+  };
+}