Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
68 changes: 68 additions & 0 deletions app/util/secure-file-reader.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,68 @@
const fs = require('fs');
const path = require('path');

const containsPath = (basePath, targetPath) => {
const relativePath = path.relative(basePath, targetPath);

return relativePath === '' || (
!relativePath.startsWith('..') &&
!path.isAbsolute(relativePath)
);
};

const resolveContainedFile = (baseDirectory, fileName) => {
if (path.isAbsolute(fileName)) {
throw new Error('File path must be relative to the configured directory');
}

const resolvedBaseDirectory = path.resolve(baseDirectory);
const baseStats = fs.lstatSync(resolvedBaseDirectory);

if (baseStats.isSymbolicLink() || !baseStats.isDirectory()) {
throw new Error('Configured directory must be a real directory');
}

const resolvedFilePath = path.resolve(resolvedBaseDirectory, fileName);

if (!containsPath(resolvedBaseDirectory, resolvedFilePath)) {
throw new Error('File path must stay within the configured directory');
}

const fileStats = fs.lstatSync(resolvedFilePath);

if (fileStats.isSymbolicLink() || !fileStats.isFile()) {
throw new Error('Configured file must be a regular file');
}

const realBaseDirectory = fs.realpathSync(resolvedBaseDirectory);
const realFilePath = fs.realpathSync(resolvedFilePath);

if (!containsPath(realBaseDirectory, realFilePath)) {
throw new Error('File path must resolve within the configured directory');
}

return realFilePath;
};

const readContainedFile = (baseDirectory, fileName) => {
const filePath = resolveContainedFile(baseDirectory, fileName);
const noFollowFlag = fs.constants.O_NOFOLLOW || 0;
const fd = fs.openSync(filePath, fs.constants.O_RDONLY | noFollowFlag);

try {
const fileStats = fs.fstatSync(fd);

if (!fileStats.isFile()) {
throw new Error('Configured file must be a regular file');
}

return fs.readFileSync(fd);
} finally {
fs.closeSync(fd);
}
};

module.exports = {
readContainedFile,
resolveContainedFile
};
10 changes: 8 additions & 2 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -94,10 +94,16 @@
"cross-spawn": "^7.0.6",
"braces": "^3.0.3",
"brace-expansion": "^1.1.12",
"picomatch": "^2.3.2"
"basic-ftp": "^5.3.1",
"picomatch": "^2.3.2",
"tar-fs": "^3.1.1",
"tmp": "^0.2.5"
},
"overrides": {
"picomatch": "^2.3.2"
"basic-ftp": "^5.3.1",
"picomatch": "^2.3.2",
"tar-fs": "^3.1.1",
"tmp": "^0.2.5"
},
"packageManager": "yarn@4.14.1"
}
6 changes: 3 additions & 3 deletions server.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ let debug = require('debug')('ccd-api-gateway-web:server');
let http = require('http');
let https = require('https');
let path = require('path');
let fs = require('fs');
const {readContainedFile} = require('./app/util/secure-file-reader');

/**
* Get port from environment and store in Express.
Expand All @@ -30,8 +30,8 @@ function createServer(app) {
if (process.env.ENV === 'localdev') {
const sslDirectory = path.join(__dirname, '..', 'app', 'resources', 'localhost-ssl');
const sslOptions = {
cert: fs.readFileSync(path.join(sslDirectory, 'localhost.crt')),
key: fs.readFileSync(path.join(sslDirectory, 'localhost.key')),
cert: readContainedFile(sslDirectory, 'localhost.crt'),
key: readContainedFile(sslDirectory, 'localhost.key'),
secureProtocol: 'TLS_method'
};
return https.createServer(sslOptions, app);
Expand Down
58 changes: 58 additions & 0 deletions test/util/secure-file-reader.spec.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
const chai = require('chai');
const expect = chai.expect;
const fs = require('fs');
const os = require('os');
const path = require('path');

const secureFileReader = require('../../app/util/secure-file-reader');

describe('secure file reader', () => {
let tempDirectory;
let outsideDirectory;

beforeEach(() => {
tempDirectory = fs.mkdtempSync(path.join(os.tmpdir(), 'secure-file-reader-'));
outsideDirectory = fs.mkdtempSync(path.join(os.tmpdir(), 'secure-file-reader-outside-'));
});

afterEach(() => {
fs.rmSync(tempDirectory, {recursive: true, force: true});
fs.rmSync(outsideDirectory, {recursive: true, force: true});
});

it('should read files contained by the configured directory', () => {
fs.writeFileSync(path.join(tempDirectory, 'allowed.txt'), 'safe');

const content = secureFileReader.readContainedFile(tempDirectory, 'allowed.txt');

expect(content.toString()).to.equal('safe');
});

it('should reject path traversal outside the configured directory', () => {
fs.writeFileSync(path.join(outsideDirectory, 'outside.txt'), 'outside');
const traversalPath = path.join('..', path.basename(outsideDirectory), 'outside.txt');

expect(() => secureFileReader.readContainedFile(tempDirectory, traversalPath))
.to.throw('File path must stay within the configured directory');
});

it('should reject symbolic links', function () {
const targetPath = path.join(outsideDirectory, 'outside.txt');
const linkPath = path.join(tempDirectory, 'linked.txt');

fs.writeFileSync(targetPath, 'outside');

try {
fs.symlinkSync(targetPath, linkPath);
} catch (err) {
if (err.code === 'EPERM') {
this.skip();
}

throw err;
}

expect(() => secureFileReader.readContainedFile(tempDirectory, 'linked.txt'))
.to.throw('Configured file must be a regular file');
});
});
33 changes: 12 additions & 21 deletions yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -1889,10 +1889,10 @@ __metadata:
languageName: node
linkType: hard

"basic-ftp@npm:^5.0.2":
version: 5.0.5
resolution: "basic-ftp@npm:5.0.5"
checksum: 10/3dc56b2092b10d67e84621f5b9bbb0430469499178e857869194184d46fbdd367a9aa9fad660084388744b074b5f540e6ac8c22c0826ebba4fcc86a9d1c324e2
"basic-ftp@npm:^5.3.1":
version: 5.3.1
resolution: "basic-ftp@npm:5.3.1"
checksum: 10/9232ee155114efafadf5adee86a6750208653a9071e53e9803dceac61a66b3ba3974771ff2490ff28f1a118fdfb806ffcc488f64609e61a9cedb52480b312843
languageName: node
linkType: hard

Expand Down Expand Up @@ -5014,13 +5014,6 @@ __metadata:
languageName: node
linkType: hard

"os-tmpdir@npm:~1.0.2":
version: 1.0.2
resolution: "os-tmpdir@npm:1.0.2"
checksum: 10/5666560f7b9f10182548bf7013883265be33620b1c1b4a4d405c25be2636f970c5488ff3e6c48de75b55d02bde037249fe5dbfbb4c0fb7714953d56aed062e6d
languageName: node
linkType: hard

"otp@npm:^0.1.3":
version: 0.1.3
resolution: "otp@npm:0.1.3"
Expand Down Expand Up @@ -6254,9 +6247,9 @@ __metadata:
languageName: node
linkType: hard

"tar-fs@npm:^3.0.6":
version: 3.0.9
resolution: "tar-fs@npm:3.0.9"
"tar-fs@npm:^3.1.1":
version: 3.1.2
resolution: "tar-fs@npm:3.1.2"
dependencies:
bare-fs: "npm:^4.0.1"
bare-path: "npm:^3.0.0"
Expand All @@ -6267,7 +6260,7 @@ __metadata:
optional: true
bare-path:
optional: true
checksum: 10/00e194ef36ced339000099c3cb5205fcd9636a531158d73e0fc1ca056fbcf8dcf39a398cbc71f030bbf938d4f477f47e2913c6e37e9c007bad31e0768a120590
checksum: 10/b358fb7061eebb42bfa6f122cf62d1bdd40dc619117863f3b59eeaa4f880dc03707014905bdb592e77176703d9045956d1ba27adda4458805f9f7cbf62015cbd
languageName: node
linkType: hard

Expand Down Expand Up @@ -6332,12 +6325,10 @@ __metadata:
languageName: node
linkType: hard

"tmp@npm:^0.0.33":
version: 0.0.33
resolution: "tmp@npm:0.0.33"
dependencies:
os-tmpdir: "npm:~1.0.2"
checksum: 10/09c0abfd165cff29b32be42bc35e80b8c64727d97dedde6550022e88fa9fd39a084660415ed8e3ebaa2aca1ee142f86df8b31d4196d4f81c774a3a20fd4b6abf
"tmp@npm:^0.2.5":
version: 0.2.5
resolution: "tmp@npm:0.2.5"
checksum: 10/dd4b78b32385eab4899d3ae296007b34482b035b6d73e1201c4a9aede40860e90997a1452c65a2d21aee73d53e93cd167d741c3db4015d90e63b6d568a93d7ec
languageName: node
linkType: hard

Expand Down