const downloadFile = blobstoreRouter.get('/blobstore/download/:filename', (req, res) => {
var localFile = path.join(__dirname, '..', escape(req.params.filename));
var file = require('fs').createWriteStream(localFile);
try {
s3.getObject({
Bucket: process.env.BUCKET,
Key: req.params.filename
}).createReadStream().pipe(file);
fs.readdir('src', (_err, files) => {
files.forEach(file => {
console.log(file);
logger.info(file);
});
});
res.setHeader('Strict-Transport-Security',
'max-age=31536000; includeSubDomains');
res.sendFile(file);
} catch (err) {
logger.error('Error downloading the file ' + err);
res.send('Failed');
}
});
Checkmarx gives warning at the line res.sendFile(file);
I've added the HSTS header also for the same but not sure if that works as well. Please guide me through both. Thanks in advance
Neither adding HSTS or escaping the req.params.filename will mitigate the Path Traversal vulnerability. To understand what Path Traversal is, here's a blog post that you might want to read:
https://nodejs.org/en/knowledge/file-system/security/introduction/
There are plenty of ways to resolve this security finding, but Checkmarx recognizes the use of sanitization methods such as replace()
Remove potentially malicious characters that will allow an attacker to traverse on different paths of your S3 bucket:
var localFile = path.join(__dirname, '..', escape(req.params.filename.replace(/^(\.\.(\/|\\|$))+/, '')));
var file = require('fs').createWriteStream(localFile);
try {
s3.getObject({
Bucket: process.env.BUCKET,
Key: req.params.filename.replace(/^(\.\.(\/|\\|$))+/, '')
}).createReadStream().pipe(file);
fs.readdir('src', (_err, files) => {
files.forEach(file => {
console.log(file);
logger.info(file);
});
});