fix(cloudfunc) XSS vulnerability: html in file name: allows executing…

… malicious javascript code in the user's browser
coderaiser · Apr 23, 2018 · 23f4d47 · 23f4d47
1 parent c413d0b
commit 23f4d47
Show file tree

Hide file tree

Showing 3 changed files with 97 additions and 24 deletions.
diff --git a/client/dom/index.js b/client/dom/index.js
@@ -10,9 +10,10 @@ const Util = require('../../common/util');
 const {
     getTitle,
     FS,
-    Entity,
 } = require('../../common/cloudfunc');
 
+const {encode} = require('../../common/entity');
+
 const DOMTree = require('./dom-tree');
 
 const DOM = Object.assign({}, DOMTree, new CmdProto());
@@ -747,7 +748,7 @@ function CmdProto() {
         const dir = PREFIX + FS + Info.dirPath;
 
         link.title      = name;
-        link.innerHTML  = Entity.encode(name);
+        link.innerHTML  = encode(name);
         link.href       = dir + name;
 
         current.setAttribute('data-name', 'js-file-' + name);

diff --git a/common/cloudfunc.js b/common/cloudfunc.js
@@ -3,7 +3,7 @@
 const rendy = require('rendy');
 const currify = require('currify/legacy');
 const store = require('fullstore/legacy');
-const Entity = require('./entity');
+const encode = require('./entity').encode;
 
 const getHeaderField = currify(_getHeaderField);
 
@@ -20,7 +20,6 @@ Path('/');
 module.exports.FS = FS;
 module.exports.apiURL = '/api/v1';
 module.exports.MAX_FILE_SIZE = 500 * 1024;
-module.exports.Entity = Entity;
 module.exports.getHeaderField = getHeaderField;
 module.exports.getPathLink = getPathLink;
 module.exports.getDotDot = getDotDot;
@@ -181,7 +180,8 @@ module.exports.buildFromJSON = (params) => {
     }
 
     fileTable += files.map((file) => {
-        const link = prefix + FS + path + file.name;
+        const name = encode(file.name);
+        const link = prefix + FS + path + name;
 
         const type = getType(file.size);
         const size = getSize(file.size);
@@ -192,13 +192,13 @@ module.exports.buildFromJSON = (params) => {
 
         const linkResult = rendy(templateLink, {
             link,
-            title: file.name,
-            name: Entity.encode(file.name),
+            title: name,
+            name,
             attribute: getAttribute(file.size)
         });
 
-        const dataName = 'data-name="js-file-' + file.name + '" ';
-        const attribute = 'draggable="true" ' + dataName;
+        const dataName = `data-name="js-file-${name}" `;
+        const attribute = `draggable="true" ${dataName}`;
 
         return rendy(templateFile, {
             tag: 'li',

diff --git a/test/server/route.js b/test/server/route.js
@@ -6,17 +6,20 @@ const test = require('tape');
 const {promisify} = require('es6-promisify');
 const pullout = require('pullout');
 const request = require('request');
+const mockRequire = require('mock-require');
+const clear = require('clear-module');
 
 const rootDir = '../..';
 
 const routePath = `${rootDir}/server/route`;
+const beforePath = '../before';
 
 const {
     _getIndexPath,
 } = require(routePath);
 
 const route = require(routePath);
-const {connect}= require('../before');
+const {connect} = require(beforePath);
 
 const warp = (fn, ...a) => (...b) => fn(...b, ...a);
 const _pullout = promisify(pullout);
@@ -78,20 +81,6 @@ test('cloudcmd: route: buttons: console', async (t) => {
     done();
 });
 
-test('cloudcmd: route: buttons: no terminal', async (t) => {
-    const config = {
-        terminal: false
-    };
-
-    const {port, done} = await connect({config});
-    const result = await getStr(`http://localhost:${port}`);
-
-    t.ok(/icon-terminal none/.test(result), 'should hide terminal');
-    t.end();
-
-    done();
-});
-
 test('cloudcmd: route: buttons: no config', async (t) => {
     const config = {
         configDialog: false
@@ -286,8 +275,91 @@ test('cloudcmd: route: sendIndex: error', async (t) => {
     const data = await getStr(`http://localhost:${port}`);
 
     t.equal(data, error.message, 'should return error');
+
+    done();
+    t.end();
+});
+
+test('cloudcmd: route: sendIndex: encode', async (t) => {
+    const name = '"><svg onload=alert(3);>';
+    const nameEncoded = '&quot;&gt;&lt;svg&nbsp;onload=alert(3);&gt;';
+    const files = [{
+        name,
+    }];
+
+    const read = (path, fn) => fn(null, {
+        path,
+        files,
+    });
+
+    mockRequire('flop', {
+        read
+    });
+
+    clear(routePath);
+    clear('../../server/cloudcmd');
+    clear(beforePath);
+
+    const {connect} = require(beforePath);
+    const {port, done} = await connect();
+    const data = await getStr(`http://localhost:${port}`);
+
+    t.ok(data.includes(nameEncoded), 'should encode name');
+
+    clear('flop');
+    clear(routePath);
+    clear('../../server/cloudcmd');
+    clear(beforePath);
+
+    done();
     t.end();
+});
+
+test('cloudcmd: route: sendIndex: encode', async (t) => {
+    const name = '"><svg onload=alert(3);>';
+    const files = [{
+        name,
+    }];
+
+    const read = (path, fn) => fn(null, {
+        path,
+        files,
+    });
+
+    mockRequire('flop', {
+        read
+    });
+
+    clear(routePath);
+    clear('../../server/cloudcmd');
+    clear(beforePath);
+
+    const {connect} = require(beforePath);
+    const {port, done} = await connect();
+    const data = await getStr(`http://localhost:${port}`);
+
+    t.notOk(data.includes(name), 'should put not encoded name');
+
+    clear('flop');
+    clear(routePath);
+    clear('../../server/cloudcmd');
+    clear(beforePath);
 
     done();
+    t.end();
+});
+
+test('cloudcmd: route: buttons: no terminal', async (t) => {
+    const config = {
+        terminal: false
+    };
+
+    const {port, done} = await connect({config});
+    const result = await getStr(`http://localhost:${port}`);
+
+    t.ok(/icon-terminal none/.test(result), 'should hide terminal');
+
+    done();
+    t.end();
 });