diff --git a/app/attachments.js b/app/attachments.js index 9ef5b7949..0966bc906 100644 --- a/app/attachments.js +++ b/app/attachments.js @@ -37,7 +37,11 @@ exports.createReader = root => { } const absolutePath = path.join(root, relativePath); - const buffer = await fse.readFile(absolutePath); + const normalized = path.normalize(absolutePath); + if (!normalized.startsWith(root)) { + throw new Error('Invalid relative path'); + } + const buffer = await fse.readFile(normalized); return toArrayBuffer(buffer); }; }; @@ -83,8 +87,13 @@ exports.createWriterForExisting = root => { const buffer = Buffer.from(arrayBuffer); const absolutePath = path.join(root, relativePath); - await fse.ensureFile(absolutePath); - await fse.writeFile(absolutePath, buffer); + const normalized = path.normalize(absolutePath); + if (!normalized.startsWith(root)) { + throw new Error('Invalid relative path'); + } + + await fse.ensureFile(normalized); + await fse.writeFile(normalized, buffer); return relativePath; }; }; @@ -103,6 +112,10 @@ exports.createDeleter = root => { } const absolutePath = path.join(root, relativePath); + const normalized = path.normalize(absolutePath); + if (!normalized.startsWith(root)) { + throw new Error('Invalid relative path'); + } await fse.remove(absolutePath); }; }; @@ -124,5 +137,11 @@ exports.getRelativePath = name => { }; // createAbsolutePathGetter :: RoothPath -> RelativePath -> AbsolutePath -exports.createAbsolutePathGetter = rootPath => relativePath => - path.join(rootPath, relativePath); +exports.createAbsolutePathGetter = rootPath => relativePath => { + const absolutePath = path.join(rootPath, relativePath); + const normalized = path.normalize(absolutePath); + if (!normalized.startsWith(rootPath)) { + throw new Error('Invalid relative path'); + } + return normalized; +}; diff --git a/test/app/attachments_test.js b/test/app/attachments_test.js index 3abf3631c..70932750f 100644 --- a/test/app/attachments_test.js +++ b/test/app/attachments_test.js @@ -77,6 +77,28 @@ describe('Attachments', () => { const inputBuffer = Buffer.from(input); assert.deepEqual(inputBuffer, output); }); + + it('throws if relative path goes higher than root', async () => { + const input = stringToArrayBuffer('test string'); + const tempDirectory = path.join( + tempRootDirectory, + 'Attachments_createWriterForExisting' + ); + + const relativePath = '../../parent'; + const attachment = { + path: relativePath, + data: input, + }; + try { + await Attachments.createWriterForExisting(tempDirectory)(attachment); + } catch (error) { + assert.strictEqual(error.message, 'Invalid relative path'); + return; + } + + throw new Error('Expected an error'); + }); }); describe('createReader', () => { @@ -110,6 +132,24 @@ describe('Attachments', () => { assert.deepEqual(input, output); }); + + it('throws if relative path goes higher than root', async () => { + const tempDirectory = path.join( + tempRootDirectory, + 'Attachments_createReader' + ); + + const relativePath = '../../parent'; + + try { + await Attachments.createReader(tempDirectory)(relativePath); + } catch (error) { + assert.strictEqual(error.message, 'Invalid relative path'); + return; + } + + throw new Error('Expected an error'); + }); }); describe('createDeleter', () => { @@ -142,6 +182,24 @@ describe('Attachments', () => { const existsFile = await fse.exists(fullPath); assert.isFalse(existsFile); }); + + it('throws if relative path goes higher than root', async () => { + const tempDirectory = path.join( + tempRootDirectory, + 'Attachments_createDeleter' + ); + + const relativePath = '../../parent'; + + try { + await Attachments.createDeleter(tempDirectory)(relativePath); + } catch (error) { + assert.strictEqual(error.message, 'Invalid relative path'); + return; + } + + throw new Error('Expected an error'); + }); }); describe('createName', () => { @@ -157,4 +215,30 @@ describe('Attachments', () => { assert.lengthOf(Attachments.getRelativePath(name), PATH_LENGTH); }); }); + + describe('createAbsolutePathGetter', () => { + it('combines root and relative path', () => { + const root = '/tmp'; + const relative = 'ab/abcdef'; + const pathGetter = Attachments.createAbsolutePathGetter(root); + const absolutePath = pathGetter(relative); + + assert.strictEqual(absolutePath, '/tmp/ab/abcdef'); + }); + + it('throws if relative path goes higher than root', () => { + const root = '/tmp'; + const relative = '../../ab/abcdef'; + const pathGetter = Attachments.createAbsolutePathGetter(root); + + try { + pathGetter(relative); + } catch (error) { + assert.strictEqual(error.message, 'Invalid relative path'); + return; + } + + throw new Error('Expected an error'); + }); + }); });