Skip to content

Commit

Permalink
fix: escape special characters in filenames (#1147)
Browse files Browse the repository at this point in the history
* fix: escape special characters in zip filenames

* Applied filename substitution to individual asset exports, tweaked client regex when saving filename to prevent it from cutting off the suggested filename after a semicolon (semicolon is used in the filename character substitutions)

* tests for filename reserved chars substitution

* removed label property from test asset creation

---------

Co-authored-by: csmig <csmig@csmig.com>
Co-authored-by: cd-rite <61710958+cd-rite@users.noreply.github.com>
  • Loading branch information
3 people authored Nov 14, 2023
1 parent 30d8daf commit 698a94f
Show file tree
Hide file tree
Showing 6 changed files with 499 additions and 29 deletions.
17 changes: 12 additions & 5 deletions api/source/controllers/Asset.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,15 @@

const writer = require('../utils/writer');
const config = require('../utils/config')
const escape = require('../utils/escape')
const AssetService = require(`../service/${config.database.type}/AssetService`);
const CollectionService = require(`../service/${config.database.type}/CollectionService`);
const dbUtils = require(`../service/${config.database.type}/utils`)
const {XMLBuilder} = require("fast-xml-parser")
const SmError = require('../utils/error')
const {escapeForXml} = require('../utils/escape')


module.exports.createAsset = async function createAsset (req, res, next) {
try {
const elevate = req.query.elevate
Expand Down Expand Up @@ -284,7 +286,8 @@ module.exports.getChecklistByAssetStig = async function getChecklistByAssetStig
}
else if (format === 'cklb') {
response.cklb.title = `${response.assetName}-${benchmarkId}-${response.revisionStrResolved}`
writer.writeInlineFile(res, JSON.stringify(response.cklb), `${response.assetName}-${benchmarkId}-${response.revisionStrResolved}.cklb`, 'application/json') // revisionStrResolved provides specific rev string, if "latest" was asked for.
let filename = escape.escapeFilename(`${response.assetName}-${benchmarkId}-${response.revisionStrResolved}.cklb`)
writer.writeInlineFile(res, JSON.stringify(response.cklb), filename, 'application/json') // revisionStrResolved provides specific rev string, if "latest" was asked for.
}
else if (format === 'ckl') {
const builder = new XMLBuilder({
Expand All @@ -300,7 +303,8 @@ module.exports.getChecklistByAssetStig = async function getChecklistByAssetStig
})
let xml = `<?xml version="1.0" encoding="UTF-8"?>\n<!-- STIG Manager ${config.version} -->\n<!-- Classification: ${config.settings.setClassification} -->\n`
xml += builder.build(response.xmlJs)
writer.writeInlineFile(res, xml, `${response.assetName}-${benchmarkId}-${response.revisionStrResolved}.ckl`, 'application/xml') // revisionStrResolved provides specific rev string, if "latest" was asked for.
let filename = escape.escapeFilename(`${response.assetName}-${benchmarkId}-${response.revisionStrResolved}.ckl`)
writer.writeInlineFile(res, xml, filename, 'application/xml') // revisionStrResolved provides specific rev string, if "latest" was asked for.
}
else if (format === 'xccdf') {
const builder = new XMLBuilder({
Expand All @@ -318,7 +322,8 @@ module.exports.getChecklistByAssetStig = async function getChecklistByAssetStig
})
let xml = `<?xml version="1.0" encoding="UTF-8"?>\n<!-- STIG Manager ${config.version} -->\n<!-- Classification: ${config.settings.setClassification} -->\n`
xml += builder.build(response.xmlJs)
writer.writeInlineFile(res, xml, `${response.assetName}-${benchmarkId}-${response.revisionStrResolved}-xccdf.xml`, 'application/xml') // revisionStrResolved provides specific rev string, if "latest" was asked for.
let filename = escape.escapeFilename(`${response.assetName}-${benchmarkId}-${response.revisionStrResolved}-xccdf.xml`)
writer.writeInlineFile(res, xml, filename, 'application/xml') // revisionStrResolved provides specific rev string, if "latest" was asked for.
}
}
else {
Expand Down Expand Up @@ -358,7 +363,8 @@ module.exports.getChecklistByAsset = async function getChecklistByAssetStig (req
const response = await AssetService.getChecklistByAsset(assetId, stigs, format, false, req.userObject )

if (format === 'cklb') {
writer.writeInlineFile(res, JSON.stringify(response.cklb), `${response.assetName}.cklb`, 'application/json')
let filename = escape.escapeFilename(`${response.assetName}.cklb`)
writer.writeInlineFile(res, JSON.stringify(response.cklb), filename, 'application/json')
}
else if (format === 'ckl') {
const builder = new XMLBuilder({
Expand All @@ -374,7 +380,8 @@ module.exports.getChecklistByAsset = async function getChecklistByAssetStig (req
})
let xml = `<?xml version="1.0" encoding="UTF-8"?>\n<!-- STIG Manager ${config.version} -->\n`
xml += builder.build(response.xmlJs)
writer.writeInlineFile(res, xml, `${response.assetName}.ckl`, 'application/xml')
let filename = escape.escapeFilename(`${response.assetName}.ckl`)
writer.writeInlineFile(res, xml, filename, 'application/xml')
}
}
catch (err) {
Expand Down
6 changes: 5 additions & 1 deletion api/source/controllers/Collection.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

const writer = require('../utils/writer')
const config = require('../utils/config')
const escape = require('../utils/escape')
const CollectionService = require(`../service/${config.database.type}/CollectionService`)
const AssetService = require(`../service/${config.database.type}/AssetService`)
const STIGService = require(`../service/${config.database.type}/STIGService`)
Expand Down Expand Up @@ -708,7 +709,9 @@ async function postArchiveByCollection ({format = 'ckl-mono', req, res, parsedRe
attrValueProcessor: escapeForXml
})
const zip = Archiver('zip', {zlib: {level: 9}})
res.attachment(`${parsedRequest.collection.name}-${format.startsWith('ckl-') ? 'CKL' : format.startsWith('cklb-') ? 'CKLB' : 'XCCDF'}.zip`)
const attachmentName = escape.escapeFilename(`${parsedRequest.collection.name}-${format.startsWith('ckl-') ?
'CKL' : format.startsWith('cklb-') ? 'CKLB' : 'XCCDF'}.zip`)
res.attachment(attachmentName)
zip.pipe(res)
const manifest = {
started: new Date().toISOString(),
Expand Down Expand Up @@ -755,6 +758,7 @@ async function postArchiveByCollection ({format = 'ckl-mono', req, res, parsedRe
filename += `-${arg.stigs[0].benchmarkId}-${response.revisionStrResolved}`
}
filename += `${format === 'xccdf' ? '-xccdf.xml' : format.startsWith('ckl-') ? '.ckl' : '.cklb'}`
filename = escape.escapeFilename(filename)
zip.append(data, {name: filename})
manifest.members.push(filename)
manifest.memberCount += 1
Expand Down
72 changes: 53 additions & 19 deletions api/source/utils/escape.js
Original file line number Diff line number Diff line change
@@ -1,31 +1,65 @@


/**
* Regex matches characters that need to be escaped in XML.
* @type {RegExp}
* Escapes XML reserved characters with named entity references.
* @param {string} value - The string to escape.
* @returns {string} The escaped string.
*/
const regexEscapeXml = /["&'<>]/g
module.exports.escapeForXml = function (name, value) {
/**
* Regex matches characters that need to be escaped in XML.
* @type {RegExp}
*/
const regexEscapeXml = /["&'<>]/g

/**
* Map of characters to their corresponding named XML entities.
* @type {Object.<string, string>}
*/
const escapeMapXml = {
'"': '&quot;',
'&': '&amp;',
'\'': '&apos;',
'<': '&lt;',
'>': '&gt;'
};
/**
* Map of characters to their corresponding named XML entities.
* @type {Object.<string, string>}
*/
const escapeMapXml = {
'"': '&quot;',
'&': '&amp;',
'\'': '&apos;',
'<': '&lt;',
'>': '&gt;'
}
return value.toString().replace(regexEscapeXml, function ($0) {
return escapeMapXml[$0]
})
}

/**
* Escapes XML reserved characters with named entity references.
* Escapes filesystem reserved characters with named entity references.
* @param {string} value - The string to escape.
* @returns {string} The escaped string.
*/
module.exports.escapeForXml = function(name, value) {
return value.toString().replace(regexEscapeXml, function($0) {
return escapeMapXml[$0]
});
module.exports.escapeFilename = function (value) {
/**
* Regexes match characters that need to be escaped in filenames.
* @type {RegExp}
*/
const osReserved = /[\/\\:\*"\?<>\|]/g
const controlChars = /[\x00-\x1f]/g

/**
* Map of characters to their corresponding named HTML entities.
* @type {Object.<string, string>}
*/
const osReserveReplace = {
'/': '&sol;',
'\\': '&bsol;',
':': '&colon;',
'*': '&ast;',
'"': '&quot;',
'?': '&quest;',
'<': '&lt;',
'>': '&gt;',
'|': '&vert;',
}

return value.toString()
.replace(osReserved, (match) => osReserveReplace[match])
.replace(controlChars, (match) => `&#x${match.charCodeAt(0).toString().padStart(2,'0')};`)
.substring(0, 255)
}

2 changes: 1 addition & 1 deletion client/src/js/SM/Exports.js
Original file line number Diff line number Diff line change
Expand Up @@ -750,7 +750,7 @@ SM.Exports.exportArchiveStreaming = async function ({collectionId, checklists, f
}
initProgress("Downloading checklists", "Initializing...")
updateStatusText(`When the stream has finished you will be prompted to save the data to disk. The final size of the archive is unknown during streaming.`, true)
const filename = contentDisposition.match(/filename\*?=['"]?(?:UTF-\d['"]*)?([^;\r\n"']*)['"]?;?/)[1]
const filename = contentDisposition.match(/filename\*?=['"]?(?:UTF-\d['"]*)?([^\r\n"']*)['"]?;?/)[1]
const reader = response.body.getReader()
let receivedLength = 0; // received that many bytes at the moment
let chunks = []; // array of received binary chunks (comprises the body)
Expand Down
6 changes: 3 additions & 3 deletions client/src/js/review.js
Original file line number Diff line number Diff line change
Expand Up @@ -219,7 +219,7 @@ async function addReview( params ) {
xhr.onload = function () {
if (this.status >= 200 && this.status < 300) {
var contentDispo = this.getResponseHeader('Content-Disposition')
var fileName = contentDispo.match(/filename\*?=['"]?(?:UTF-\d['"]*)?([^;\r\n"']*)['"]?;?/)[1]
var fileName = contentDispo.match(/filename\*?=['"]?(?:UTF-\d['"]*)?([^\r\n"']*)['"]?;?/)[1]
resolve({
blob: xhr.response,
filename: fileName
Expand Down Expand Up @@ -267,7 +267,7 @@ async function addReview( params ) {
xhr.onload = function () {
if (this.status >= 200 && this.status < 300) {
var contentDispo = this.getResponseHeader('Content-Disposition')
var fileName = contentDispo.match(/filename\*?=['"]?(?:UTF-\d['"]*)?([^;\r\n"']*)['"]?;?/)[1]
var fileName = contentDispo.match(/filename\*?=['"]?(?:UTF-\d['"]*)?([^\r\n"']*)['"]?;?/)[1]
resolve({
blob: xhr.response,
filename: fileName
Expand Down Expand Up @@ -314,7 +314,7 @@ async function addReview( params ) {
})
const contentDispo = response.headers.get("content-disposition")
if (contentDispo) {
const filename = contentDispo.match(/filename\*?=['"]?(?:UTF-\d['"]*)?([^;\r\n"']*)['"]?;?/)[1]
const filename = contentDispo.match(/filename\*?=['"]?(?:UTF-\d['"]*)?([^\r\n"']*)['"]?;?/)[1]
console.log(filename)
const blob = await response.blob()
saveAs(blob, filename)
Expand Down
Loading

0 comments on commit 698a94f

Please sign in to comment.