fix: path traversal (#474)

Author: coderaiser

server/markdown/index.js

@@ -32,9 +32,9 @@ function parseName(query, name, rootDir) {
     const shortName = name.replace('/markdown', '');
 
     if (query === 'relative')
-        return DIR_ROOT + shortName;
+        return root.resolve(shortName, DIR_ROOT);
 
-    return root(shortName, rootDir);
+    return root.resolve(shortName, rootDir);
 }
 
 async function onGET(request, name, root) {

server/markdown/index.spec.js

@@ -1,5 +1,6 @@
 import fs from 'node:fs';
 import {join} from 'node:path';
+import {tmpdir} from 'node:os';
 import {promisify} from 'node:util';
 import {tryToCatch} from 'try-to-catch';
 import test from 'supertape';
@@ -126,3 +127,25 @@ test('cloudcmd: markdown: zip', async (t) => {
     t.equal(body, '<h1>hello</h1>\n');
     t.end();
 });
+
+test('cloudcmd: markdown: relative: path traversal', async (t) => {
+    const configManager = cloudcmd.createConfigManager();
+    const sandboxDir = fs.mkdtempSync(join(tmpdir(), 'cloudcmd-test-'));
+    
+    const config = {
+        auth: false,
+        root: sandboxDir,
+    };
+    
+    const {request} = serveOnce(cloudcmd, {
+        config,
+        configManager,
+    });
+    
+    const {body} = await request.get('/api/v1/markdown/..%2f..%2f..%2fetc%2fpasswd?relative');
+    
+    fs.rmdirSync(sandboxDir);
+    
+    t.match(body, 'beyond root', 'should reject url-encoded traversal');
+    t.end();
+});

server/rest/index.js

@@ -11,6 +11,7 @@ import json from 'jonny';
 import * as ponse from 'ponse';
 import {copymitter} from 'copymitter';
 import {moveFiles as _moveFiles} from '@cloudcmd/move-files';
+import {tryCatch} from 'try-catch';
 import * as CloudFunc from '#common/cloudfunc';
 import root from '../root.js';
 import markdown from '../markdown/index.js';
@@ -216,8 +217,15 @@ function onPUT({name, fs, moveFiles, config, body}, callback) {
         const msg = getMoveMsg(names);
         const fn = swap(callback, msg);
 
-        const fromRooted = root(from, rootDir);
-        const toRooted = root(to, rootDir);
+        const [errorFrom, fromRooted] = tryCatch(root.resolve, from, rootDir);
+        
+        if (errorFrom)
+            return callback(errorFrom);
+        
+        const [errorTo, toRooted] = tryCatch(root.resolve, to, rootDir);
+        
+        if (errorTo)
+            return callback(errorTo);
 
         return moveFiles(fromRooted, toRooted, names)
             .on('error', fn)
@@ -227,21 +235,32 @@ function onPUT({name, fs, moveFiles, config, body}, callback) {
     case 'rename':
         return rename(rootDir, files.from, files.to, fs, callback);
 
-    case 'copy':
+    case 'copy': {
         if (!files.from || !files.names || !files.to)
             return callback(body);
 
         if (isRootAll(rootDir, [files.to, files.from]))
             return callback(getWin32RootMsg());
 
-        files.from = root(files.from, rootDir);
-        files.to = root(files.to, rootDir);
+        const [errorFrom, resolvedFrom] = tryCatch(root.resolve, files.from, rootDir);
+        
+        if (errorFrom)
+            return callback(errorFrom);
+        
+        const [errorTo, resolvedTo] = tryCatch(root.resolve, files.to, rootDir);
+        
+        if (errorTo)
+            return callback(errorTo);
+        
+        files.from = resolvedFrom;
+        files.to = resolvedTo;
 
         copy(files.from, files.to, files.names, (error) => {
             const msg = formatMsg('copy', files.names);
             callback(error, msg);
         });
         break;
+    }
 
     case 'pack':
         if (!files.from)
@@ -274,8 +293,15 @@ function rename(rootDir, from, to, fs, callback) {
     const msg = getRenameMsg(from, to);
     const fn = swap(callback, msg);
 
-    const fromRooted = root(from, rootDir);
-    const toRooted = root(to, rootDir);
+    const [errorFrom, fromRooted] = tryCatch(root.resolve, from, rootDir);
+    
+    if (errorFrom)
+        return callback(errorFrom);
+    
+    const [errorTo, toRooted] = tryCatch(root.resolve, to, rootDir);
+    
+    if (errorTo)
+        return callback(errorTo);
 
     return fs.rename(fromRooted, toRooted, fn);
 }
@@ -286,8 +312,18 @@ function pack(from, to, names, config, fn) {
     const rootDir = config('root');
     const packer = config('packer');
 
-    from = root(from, rootDir);
-    to = root(to, rootDir);
+    const [errorFrom, resolvedFrom] = tryCatch(root.resolve, from, rootDir);
+    
+    if (errorFrom)
+        return fn(errorFrom);
+    
+    const [errorTo, resolvedTo] = tryCatch(root.resolve, to, rootDir);
+    
+    if (errorTo)
+        return fn(errorTo);
+    
+    from = resolvedFrom;
+    to = resolvedTo;
 
     if (!names) {
         names = [
@@ -305,12 +341,23 @@ export const _pack = pack;
 function extract(from, to, config, fn) {
     const rootDir = config('root');
 
-    from = root(from, rootDir);
+    const [errorFrom, resolvedFrom] = tryCatch(root.resolve, from, rootDir);
 
-    if (to)
-        to = root(to, rootDir);
-    else
+    if (errorFrom)
+        return fn(errorFrom);
+    
+    from = resolvedFrom;
+    
+    if (to) {
+        const [errorTo, resolvedTo] = tryCatch(root.resolve, to, rootDir);
+        
+        if (errorTo)
+            return fn(errorTo);
+        
+        to = resolvedTo;
+    } else {
         to = from.replace(/\.tar\.gz$/, '');
+    }
 
     operation('extract', config('packer'), from, to, fn);
 }

server/root.js

@@ -1,5 +1,20 @@
+import path from 'node:path';
 import mellow from 'mellow';
 
-export default (dir, root, {webToWin = mellow.webToWin} = {}) => {
+const rootFn = (dir, root, {webToWin = mellow.webToWin} = {}) => {
     return webToWin(dir, root || '/');
 };
+
+rootFn.resolve = (dir, rootDir) => {
+    const normalizedRoot = path.normalize(rootDir);
+    const resolved = rootFn(dir, normalizedRoot);
+    
+    const sep = normalizedRoot.endsWith(path.sep) ? '' : path.sep;
+    
+    if (resolved !== normalizedRoot && !resolved.startsWith(normalizedRoot + sep))
+        throw Error(`Path ${resolved} beyond root ${normalizedRoot}!`);
+    
+    return resolved;
+};
+
+export default rootFn;

test/rest/copy.js

@@ -42,3 +42,45 @@ test('cloudcmd: rest: copy', async (t) => {
     t.equal(body, 'copy: ok("["copy.txt"]")', 'should return result');
     t.end();
 });
+
+test('cloudcmd: rest: copy: path traversal: from', async (t) => {
+    const configManager = cloudcmd.createConfigManager();
+    configManager('auth', false);
+    configManager('root', '/sandbox');
+    
+    const {request} = serveOnce(cloudcmd, {
+        configManager,
+    });
+    
+    const {body} = await request.put('/api/v1/copy', {
+        body: {
+            from: '../../../../etc/passwd',
+            to: '/file.txt',
+            names: ['copy.txt'],
+        },
+    });
+    
+    t.equal(body, 'Path /etc/passwd beyond root /sandbox!', 'should reject traversal in from');
+    t.end();
+});
+
+test('cloudcmd: rest: copy: path traversal: to', async (t) => {
+    const configManager = cloudcmd.createConfigManager();
+    configManager('auth', false);
+    configManager('root', '/sandbox');
+    
+    const {request} = serveOnce(cloudcmd, {
+        configManager,
+    });
+    
+    const {body} = await request.put('/api/v1/copy', {
+        body: {
+            from: '/file.txt',
+            to: '../../../../etc/cron.d/evil',
+            names: ['copy.txt'],
+        },
+    });
+    
+    t.equal(body, 'Path /etc/cron.d/evil beyond root /sandbox!', 'should reject traversal in to');
+    t.end();
+});

test/rest/extract.js

@@ -0,0 +1,22 @@
+import test from 'supertape';
+import {serveOnce} from 'serve-once';
+import {cloudcmd} from '#server/cloudcmd';
+
+test('cloudcmd: rest: extract: path traversal: from', async (t) => {
+    const configManager = cloudcmd.createConfigManager();
+    configManager('auth', false);
+    configManager('root', '/sandbox');
+    
+    const {request} = serveOnce(cloudcmd, {
+        configManager,
+    });
+    
+    const {body} = await request.put('/api/v1/extract', {
+        body: {
+            from: '../../../../tmp/cc-outside/evil.zip',
+        },
+    });
+    
+    t.equal(body, 'Path /tmp/cc-outside/evil.zip beyond root /sandbox!', 'should reject traversal in from');
+    t.end();
+});

test/rest/move.js

@@ -85,3 +85,45 @@ test('cloudcmd: rest: move: no to', async (t) => {
     t.equal(body, expected);
     t.end();
 });
+
+test('cloudcmd: rest: move: path traversal: from', async (t) => {
+    const configManager = cloudcmd.createConfigManager();
+    configManager('auth', false);
+    configManager('root', '/sandbox');
+    
+    const {request} = serveOnce(cloudcmd, {
+        configManager,
+    });
+    
+    const {body} = await request.put('/api/v1/move', {
+        body: {
+            from: '../../../../etc/passwd',
+            to: '/file.txt',
+            names: ['move.txt'],
+        },
+    });
+    
+    t.equal(body, 'Path /etc/passwd beyond root /sandbox!', 'should reject traversal in from');
+    t.end();
+});
+
+test('cloudcmd: rest: move: path traversal: to', async (t) => {
+    const configManager = cloudcmd.createConfigManager();
+    configManager('auth', false);
+    configManager('root', '/sandbox');
+    
+    const {request} = serveOnce(cloudcmd, {
+        configManager,
+    });
+    
+    const {body} = await request.put('/api/v1/move', {
+        body: {
+            from: '/file.txt',
+            to: '../../../../etc/cron.d/evil',
+            names: ['move.txt'],
+        },
+    });
+    
+    t.equal(body, 'Path /etc/cron.d/evil beyond root /sandbox!', 'should reject traversal in to');
+    t.end();
+});

test/rest/pack.js

@@ -229,3 +229,45 @@ const getPackOptions = (to, names = ['pack']) => ({
     names,
     from: '/fixture',
 });
+
+test('cloudcmd: rest: pack: path traversal: from', async (t) => {
+    const configManager = cloudcmd.createConfigManager();
+    configManager('auth', false);
+    configManager('root', '/sandbox');
+    
+    const {request} = serveOnce(cloudcmd, {
+        configManager,
+    });
+    
+    const {body} = await request.put('/api/v1/pack', {
+        body: {
+            from: '../../../../etc',
+            to: '/archive.zip',
+            names: ['passwd'],
+        },
+    });
+    
+    t.equal(body, 'Path /etc beyond root /sandbox!', 'should reject traversal in from');
+    t.end();
+});
+
+test('cloudcmd: rest: pack: path traversal: to', async (t) => {
+    const configManager = cloudcmd.createConfigManager();
+    configManager('auth', false);
+    configManager('root', '/sandbox');
+    
+    const {request} = serveOnce(cloudcmd, {
+        configManager,
+    });
+    
+    const {body} = await request.put('/api/v1/pack', {
+        body: {
+            from: '/files',
+            to: '../../../../tmp/cc-outside/evil.zip',
+            names: ['file.txt'],
+        },
+    });
+    
+    t.equal(body, 'Path /tmp/cc-outside/evil.zip beyond root /sandbox!', 'should reject traversal in to');
+    t.end();
+});