diff --git a/README.md b/README.md index 7e0d6f5290..cf7a139a9c 100644 --- a/README.md +++ b/README.md @@ -1566,6 +1566,8 @@ validate threshold Validate the compliance and status counts of an HD ### Generate +NOTE: Please be aware that the `delta` and `update_controls4delta` subcommands can invoke an InSpec or CINC Auditor executable supplied through the -I/--inspecPath flag. Untrusted executable paths or arguments can create a command injection risk. Furthermore, Windows requires this invocation to use a shell which can create a risk of shell injection. The SAF CLI has been hardened so as to reduce these risks; however, as a workaround, run InSpec or CINC Auditor manually to generate the profile JSON, then provide it with the -J/--inspecJsonFile flag. + #### Delta Update an existing InSpec profile with new or updated XCCDF guidance. With -M (runMapControls), uses a 3-tier SRG/CCI requirement-first matcher for cross-vendor deltas (e.g. RHEL 9 → Amazon Linux 2023) and persists per-control match decisions into delta.json's links[] field. diff --git a/package-lock.json b/package-lock.json index 57d4f6113f..71da51165e 100644 --- a/package-lock.json +++ b/package-lock.json @@ -65,6 +65,7 @@ "open": "^11.0.0", "prompt-sync": "^4.2.0", "run-script-os": "^1.1.6", + "sanitize-filename": "^1.6.4", "table": "^6.8.1", "tmp": "^0.2.1", "tsimportlib": "^0.0.5", @@ -3105,9 +3106,6 @@ "arm64" ], "dev": true, - "libc": [ - "glibc" - ], "license": "MIT", "optional": true, "os": [ @@ -3125,9 +3123,6 @@ "arm64" ], "dev": true, - "libc": [ - "musl" - ], "license": "MIT", "optional": true, "os": [ @@ -3145,9 +3140,6 @@ "ppc64" ], "dev": true, - "libc": [ - "glibc" - ], "license": "MIT", "optional": true, "os": [ @@ -3165,9 +3157,6 @@ "s390x" ], "dev": true, - "libc": [ - "glibc" - ], "license": "MIT", "optional": true, "os": [ @@ -3185,9 +3174,6 @@ "x64" ], "dev": true, - "libc": [ - "glibc" - ], "license": "MIT", "optional": true, "os": [ @@ -3205,9 +3191,6 @@ "x64" ], "dev": true, - "libc": [ - "musl" - ], "license": "MIT", "optional": true, "os": [ @@ -10195,6 +10178,15 @@ "integrity": "sha512-YZo3K82SD7Riyi0E1EQPojLz7kpepnSQI9IyPbHHg1XXXevb5dJI7tpyN2ADxGcQbHG7vcyRHk0cbwqcQriUtg==", "license": "MIT" }, + "node_modules/sanitize-filename": { + "version": "1.6.4", + "resolved": "https://registry.npmjs.org/sanitize-filename/-/sanitize-filename-1.6.4.tgz", + "integrity": "sha512-9ZyI08PsvdQl2r/bBIGubpVdR3RR9sY6RDiWFPreA21C/EFlQhmgo20UZlNjZMMZNubusLhAQozkA0Od5J21Eg==", + "license": "WTFPL OR ISC", + "dependencies": { + "truncate-utf8-bytes": "^1.0.0" + } + }, "node_modules/sanitize-html": { "version": "2.17.6", "resolved": "https://registry.npmjs.org/sanitize-html/-/sanitize-html-2.17.6.tgz", @@ -10879,6 +10871,15 @@ "node": ">= 14.0.0" } }, + "node_modules/truncate-utf8-bytes": { + "version": "1.0.2", + "resolved": "https://registry.npmjs.org/truncate-utf8-bytes/-/truncate-utf8-bytes-1.0.2.tgz", + "integrity": "sha512-95Pu1QXQvruGEhv62XCMO3Mm90GscOCClvrIUwCM0PYOXK3kaF3l3sIHxx71ThJfcbM2O5Au6SO3AWCSEfW4mQ==", + "license": "WTFPL", + "dependencies": { + "utf8-byte-length": "^1.0.1" + } + }, "node_modules/ts-api-utils": { "version": "2.5.0", "resolved": "https://registry.npmjs.org/ts-api-utils/-/ts-api-utils-2.5.0.tgz", @@ -11138,6 +11139,12 @@ "punycode": "^2.1.0" } }, + "node_modules/utf8-byte-length": { + "version": "1.0.5", + "resolved": "https://registry.npmjs.org/utf8-byte-length/-/utf8-byte-length-1.0.5.tgz", + "integrity": "sha512-Xn0w3MtiQ6zoz2vFyUVruaCL53O/DwUvkEeOvj+uulMm0BkUGYWmBYVyElqZaSLhY6ZD0ulfU3aBra2aVT4xfA==", + "license": "(WTFPL OR MIT)" + }, "node_modules/util-deprecate": { "version": "1.0.2", "resolved": "https://registry.npmjs.org/util-deprecate/-/util-deprecate-1.0.2.tgz", diff --git a/package.json b/package.json index 9331de49df..203debcf07 100644 --- a/package.json +++ b/package.json @@ -62,6 +62,7 @@ "open": "^11.0.0", "prompt-sync": "^4.2.0", "run-script-os": "^1.1.6", + "sanitize-filename": "^1.6.4", "table": "^6.8.1", "tmp": "^0.2.1", "tsimportlib": "^0.0.5", diff --git a/src/commands/attest/apply.ts b/src/commands/attest/apply.ts index 868f52c12d..4b6bed379a 100644 --- a/src/commands/attest/apply.ts +++ b/src/commands/attest/apply.ts @@ -1,11 +1,10 @@ import fs from 'fs'; -import path from 'path'; import { Flags } from '@oclif/core'; import _ from 'lodash'; import type { ExecJSON } from 'inspecjs'; import { addAttestationToHDF, parseXLSXAttestations, type Attestation } from '@mitre/hdf-converters'; import yaml from 'yaml'; -import { basename } from '../../utils/global'; +import { basename, resolveSafeChild, safeFilename } from '../../utils/global'; import { BaseCommand } from '../../utils/oclif/base_command'; export default class ApplyAttestation extends BaseCommand { @@ -74,7 +73,7 @@ export default class ApplyAttestation extends BaseCommand { fs.mkdirSync(flags.output); _.forOwn(results, (result, filename) => { fs.writeFileSync( - path.join(flags.output, checkSuffix(basename(filename))), + resolveSafeChild(flags.output, safeFilename(checkSuffix(filename))), JSON.stringify(result, null, 2), ); }); diff --git a/src/commands/convert/ckl2poam.ts b/src/commands/convert/ckl2poam.ts index 325f1c84a6..2399cc177a 100644 --- a/src/commands/convert/ckl2poam.ts +++ b/src/commands/convert/ckl2poam.ts @@ -1,5 +1,4 @@ import { mkdir, readFile } from 'fs/promises'; -import path from 'path'; import { Flags } from '@oclif/core'; import { XMLParser } from 'fast-xml-parser'; import _ from 'lodash'; @@ -23,7 +22,7 @@ import { extractSTIGUrl, replaceSpecialCharacters, } from '../../utils/ckl2poam'; -import { basename, dataURLtoU8Array } from '../../utils/global'; +import { basename, dataURLtoU8Array, resolveSafeChild, safeFilename } from '../../utils/global'; import { createWinstonLogger } from '../../utils/logging'; import { BaseCommand } from '../../utils/oclif/base_command'; @@ -318,7 +317,7 @@ export default class CKL2POAM extends BaseCommand { currentRow += flags.rowsToSkip + 1; } } - return workBook.toFileAsync(path.join(flags.output, `${basename(fileName)}-${moment(new Date()).format('YYYY-MM-DD-HHmm')}.xlsm`)); + return workBook.toFileAsync(resolveSafeChild(flags.output, safeFilename(`${basename(fileName)}-${moment(new Date()).format('YYYY-MM-DD-HHmm')}.xlsm`))); })); } } diff --git a/src/commands/convert/conveyor2hdf.ts b/src/commands/convert/conveyor2hdf.ts index 4223ffd047..a93740c1f4 100644 --- a/src/commands/convert/conveyor2hdf.ts +++ b/src/commands/convert/conveyor2hdf.ts @@ -1,8 +1,7 @@ import { Flags } from '@oclif/core'; import fs from 'fs'; import { ConveyorResults as Mapper, INPUT_TYPES } from '@mitre/hdf-converters'; -import { basename, checkInput, checkSuffix } from '../../utils/global'; -import path from 'path'; +import { checkInput, checkSuffix, resolveSafeChild, safeFilename } from '../../utils/global'; import { BaseCommand } from '../../utils/oclif/base_command'; export default class Conveyor2HDF extends BaseCommand { static readonly usage @@ -38,7 +37,7 @@ export default class Conveyor2HDF extends BaseCommand { fs.mkdirSync(flags.output); for (const [filename, result] of Object.entries(results)) { fs.writeFileSync( - path.join(flags.output, checkSuffix(basename(filename))), + resolveSafeChild(flags.output, safeFilename(checkSuffix(filename))), JSON.stringify(result, null, 2), ); } diff --git a/src/commands/convert/hdf2asff.ts b/src/commands/convert/hdf2asff.ts index 52259a535d..7f469d2bdc 100644 --- a/src/commands/convert/hdf2asff.ts +++ b/src/commands/convert/hdf2asff.ts @@ -1,6 +1,5 @@ import fs from 'fs'; import https from 'https'; -import path from 'path'; import { type AwsSecurityFinding, SecurityHub, @@ -10,7 +9,7 @@ import { FromHdfToAsffMapper as Mapper } from '@mitre/hdf-converters'; import { Flags } from '@oclif/core'; import { NodeHttpHandler } from '@smithy/node-http-handler'; import _ from 'lodash'; -import { basename, checkSuffix } from '../../utils/global'; +import { basename, checkSuffix, resolveSafeChild, safeFilename } from '../../utils/global'; import { BaseCommand } from '../../utils/oclif/base_command'; export default class HDF2ASFF extends BaseCommand { @@ -104,9 +103,9 @@ export default class HDF2ASFF extends BaseCommand { const outputFolder = flags.output.replace('.json', '') || 'asff-output'; fs.mkdirSync(outputFolder); if (convertedSlices.length === 1) { - const outfilePath = path.join( + const outfilePath = resolveSafeChild( outputFolder, - checkSuffix(basename(flags.output)), + safeFilename(checkSuffix(flags.output)), ); fs.writeFileSync( outfilePath, @@ -114,9 +113,9 @@ export default class HDF2ASFF extends BaseCommand { ); } else { for (const [index, slice] of convertedSlices.entries()) { - const outfilePath = path.join( + const outfilePath = resolveSafeChild( outputFolder, - `${checkSuffix(basename(flags.output || '')).replace('.json', '')}.p${index}.json`, + safeFilename(`${checkSuffix(basename(flags.output || '')).replace('.json', '')}.p${index}.json`), ); fs.writeFileSync(outfilePath, JSON.stringify(slice, null, 2)); } diff --git a/src/commands/convert/hdf2csv.ts b/src/commands/convert/hdf2csv.ts index 67c701b3a2..9db308347e 100644 --- a/src/commands/convert/hdf2csv.ts +++ b/src/commands/convert/hdf2csv.ts @@ -9,7 +9,7 @@ import _ from 'lodash'; import type { Logger } from 'winston'; import type { ControlSetRows } from '../../types/csv'; import { convertRow, csvExportFields } from '../../utils/csv'; -import { basename } from '../../utils/global'; +import { basename, resolveSafeChild, safeFilename } from '../../utils/global'; import { createWinstonLogger } from '../../utils/logging'; import { BaseCommand } from '../../utils/oclif/base_command'; @@ -86,7 +86,7 @@ export default class HDF2CSV extends BaseCommand { if (flags.interactive) { const interactiveFlags = await this.getFlags(); inputFile = interactiveFlags.inputFile; - outputFile = path.join(interactiveFlags.outputDirectory, interactiveFlags.outputFileName); + outputFile = resolveSafeChild(interactiveFlags.outputDirectory, safeFilename(interactiveFlags.outputFileName)); includeFields = interactiveFlags.fields.join(','); truncateFields = Boolean(interactiveFlags.truncateFields); } else if (this.requiredFlagsProvided(flags)) { diff --git a/src/commands/convert/hdf2html.ts b/src/commands/convert/hdf2html.ts index 80e93c1193..c2a794dab5 100644 --- a/src/commands/convert/hdf2html.ts +++ b/src/commands/convert/hdf2html.ts @@ -1,7 +1,7 @@ import fs from 'fs'; -import path from 'path'; import { FileExportTypes, FromHDFToHTMLMapper as Mapper } from '@mitre/hdf-converters'; import { Command, Flags } from '@oclif/core'; +import { basename } from '../../utils/global'; export default class HDF2HTML extends Command { static readonly usage = 'convert hdf2html -i ... -o [-t ] [-h]'; @@ -22,7 +22,7 @@ export default class HDF2HTML extends Command { async run() { const { flags } = await this.parse(HDF2HTML); - const files = flags.input.map((file, i) => ({ data: fs.readFileSync(file, 'utf8'), fileName: path.basename(file), fileID: `${i}` })); + const files = flags.input.map((file, i) => ({ data: fs.readFileSync(file, 'utf8'), fileName: basename(file), fileID: `${i}` })); const converter = await new Mapper(files, FileExportTypes[flags.type as keyof typeof FileExportTypes]).toHTML(); fs.writeFileSync(flags.output, converter); diff --git a/src/commands/convert/index.ts b/src/commands/convert/index.ts index f712229a31..7871184f3b 100644 --- a/src/commands/convert/index.ts +++ b/src/commands/convert/index.ts @@ -29,7 +29,7 @@ import { ZapMapper, } from '@mitre/hdf-converters'; import { Flags } from '@oclif/core'; -import { basename, checkSuffix } from '../../utils/global'; +import { basename, checkSuffix, resolveSafeChild, safeFilename } from '../../utils/global'; import { BaseCommand } from '../../utils/oclif/base_command'; import ASFF2HDF from './asff2hdf'; import Zap2HDF from './zap2hdf'; @@ -75,7 +75,7 @@ export default class Convert extends BaseCommand { filename: basename(filePath), }); switch ( - Convert.detectedType // skipcq: JS-0047 + Convert.detectedType ) { case 'asff': { return ASFF2HDF.flags; @@ -130,7 +130,7 @@ export default class Convert extends BaseCommand { fs.mkdirSync(flags.output); _.forOwn(results, (result, filename) => { fs.writeFileSync( - path.join(flags.output, checkSuffix(basename(filename))), + resolveSafeChild(flags.output, safeFilename(checkSuffix(filename))), JSON.stringify(result, null, 2), ); }); @@ -161,7 +161,7 @@ export default class Convert extends BaseCommand { fs.mkdirSync(flags.output); for (const [filename, result] of Object.entries(results)) { fs.writeFileSync( - path.join(flags.output, checkSuffix(basename(filename))), + resolveSafeChild(flags.output, safeFilename(checkSuffix(filename))), JSON.stringify(result, null, 2), ); } @@ -241,9 +241,11 @@ export default class Convert extends BaseCommand { const result = converter.toHdf(); const pluralResults = Array.isArray(result) ? result : []; const singularResult = pluralResults.length === 0; + const outputBase = path.dirname(flags.output); + const outputPrefix = safeFilename(flags.output.replaceAll(/\.json/gi, '')); for (const element of pluralResults) { fs.writeFileSync( - `${flags.output.replaceAll(/\.json/gi, '')}-${basename(_.get(element, 'platform.target_id') || '')}.json`, + resolveSafeChild(outputBase, safeFilename(`${outputPrefix}-${basename(_.get(element, 'platform.target_id') || '')}.json`)), JSON.stringify(element, null, 2), ); } @@ -294,9 +296,9 @@ export default class Convert extends BaseCommand { fs.mkdirSync(flags.output); _.forOwn(results, (result) => { fs.writeFileSync( - path.join( + resolveSafeChild( flags.output, - basename(`${_.get(result, 'platform.target_id')}.json`), + safeFilename(`${_.get(result, 'platform.target_id')}.json`), ), JSON.stringify(result, null, 2), ); @@ -327,9 +329,11 @@ export default class Convert extends BaseCommand { const result = converter.toHdf(); const pluralResults = Array.isArray(result) ? result : []; const singularResult = pluralResults.length === 0; + const outputBase = path.dirname(flags.output); + const outputPrefix = safeFilename(flags.output.replaceAll(/\.json/gi, '')); for (const element of pluralResults) { fs.writeFileSync( - `${flags.output.replaceAll(/\.json/gi, '')}-${basename(_.get(element, 'platform.target_id') || '')}.json`, + resolveSafeChild(outputBase, safeFilename(`${outputPrefix}-${basename(_.get(element, 'platform.target_id') || '')}.json`)), JSON.stringify(element, null, 2), ); } diff --git a/src/commands/convert/ionchannel2hdf.ts b/src/commands/convert/ionchannel2hdf.ts index dad8e707aa..345b23ea1c 100644 --- a/src/commands/convert/ionchannel2hdf.ts +++ b/src/commands/convert/ionchannel2hdf.ts @@ -1,13 +1,13 @@ import { INPUT_TYPES, IonChannelAPIMapper, IonChannelMapper } from '@mitre/hdf-converters'; import { Flags } from '@oclif/core'; import { - basename, checkInput, checkSuffix, + resolveSafeChild, + safeFilename, } from '../../utils/global'; import { createWinstonLogger } from '../../utils/logging'; import fs from 'fs'; -import path from 'path'; import { BaseCommand } from '../../utils/oclif/base_command'; export default class IonChannel2HDF extends BaseCommand { @@ -107,7 +107,7 @@ export default class IonChannel2HDF extends BaseCommand { } fs.writeFileSync( - path.join(flags.output, basename(filename)), + resolveSafeChild(flags.output, safeFilename(filename)), JSON.stringify(json, null, 2), ); } @@ -134,7 +134,7 @@ export default class IonChannel2HDF extends BaseCommand { } fs.writeFileSync( - path.join(flags.output, basename(filename)), + resolveSafeChild(flags.output, safeFilename(filename)), JSON.stringify(json, null, 2), ); } @@ -152,10 +152,7 @@ export default class IonChannel2HDF extends BaseCommand { logger.debug(`Processing...${filename}`); fs.writeFileSync( - path.join( - flags.output, - checkSuffix(basename(filename)), - ), + resolveSafeChild(flags.output, safeFilename(checkSuffix(filename))), JSON.stringify(new IonChannelMapper(data).toHdf()), ); } diff --git a/src/commands/convert/msft_secure2hdf.ts b/src/commands/convert/msft_secure2hdf.ts index b72319e84f..c6d7fb56b4 100644 --- a/src/commands/convert/msft_secure2hdf.ts +++ b/src/commands/convert/msft_secure2hdf.ts @@ -1,5 +1,6 @@ import fs from 'fs'; import https from 'https'; +import path from 'path'; import { ClientSecretCredential } from '@azure/identity'; import { Client, @@ -15,7 +16,7 @@ import type { import { MsftSecureScoreResults as Mapper } from '@mitre/hdf-converters'; import { Flags } from '@oclif/core'; import type { ExecJSON } from 'inspecjs'; -import { basename } from '../../utils/global'; +import { basename, resolveSafeChild, safeFilename } from '../../utils/global'; import { BaseCommand } from '../../utils/oclif/base_command'; function processInputs( @@ -33,6 +34,8 @@ function processInputs( ); for (const hdfReport of converter.toHdf()) { + const outputBase = path.dirname(output); + const outputPrefix = safeFilename(output.replaceAll(/\.json/gi, '')); const auxData = ( (hdfReport as ExecJSON.Execution & { passthrough: Record }) .passthrough?.auxiliary_data as Record[] @@ -40,7 +43,7 @@ function processInputs( ?.data as Record; const reportId = auxData?.reportId as string; fs.writeFileSync( - `${output.replaceAll(/\.json/gi, '')}-${basename(reportId)}.json`, + resolveSafeChild(outputBase, safeFilename(`${outputPrefix}-${basename(reportId)}.json`)), JSON.stringify(hdfReport), ); } diff --git a/src/commands/convert/nessus2hdf.ts b/src/commands/convert/nessus2hdf.ts index 6b8088f984..81d3c1ab7b 100644 --- a/src/commands/convert/nessus2hdf.ts +++ b/src/commands/convert/nessus2hdf.ts @@ -1,8 +1,9 @@ import { Flags } from '@oclif/core'; import fs from 'fs'; +import path from 'path'; import { INPUT_TYPES, NessusResults as Mapper } from '@mitre/hdf-converters'; import _ from 'lodash'; -import { basename, checkInput, checkSuffix } from '../../utils/global'; +import { basename, checkInput, checkSuffix, resolveSafeChild, safeFilename } from '../../utils/global'; import { BaseCommand } from '../../utils/oclif/base_command'; export default class Nessus2HDF extends BaseCommand { @@ -48,9 +49,11 @@ export default class Nessus2HDF extends BaseCommand { const converter = new Mapper(data, flags.includeRaw); const result = await converter.toHdf(); if (Array.isArray(result)) { + const outputBase = path.dirname(flags.output); + const outputPrefix = safeFilename(flags.output.replaceAll(/\.json/gi, '')); for (const element of result) { fs.writeFileSync( - `${flags.output.replaceAll(/\.json/gi, '')}-${basename(_.get(element, 'platform.target_id') || '')}.json`, + resolveSafeChild(outputBase, safeFilename(`${outputPrefix}-${basename(_.get(element, 'platform.target_id') || '')}.json`)), JSON.stringify(element, null, 2), ); } diff --git a/src/commands/convert/prisma2hdf.ts b/src/commands/convert/prisma2hdf.ts index 0c1ed74d46..9182c3ae63 100644 --- a/src/commands/convert/prisma2hdf.ts +++ b/src/commands/convert/prisma2hdf.ts @@ -1,9 +1,8 @@ import { Flags } from '@oclif/core'; import fs from 'fs'; import { PrismaMapper as Mapper } from '@mitre/hdf-converters'; -import path from 'path'; import _ from 'lodash'; -import { basename } from '../../utils/global'; +import { resolveSafeChild, safeFilename } from '../../utils/global'; import { BaseCommand } from '../../utils/oclif/base_command'; export default class Prisma2HDF extends BaseCommand { @@ -42,7 +41,7 @@ export default class Prisma2HDF extends BaseCommand { _.forOwn(results, (result) => { fs.writeFileSync( - path.join(flags.output, basename(`${_.get(result, 'platform.target_id')}.json`)), + resolveSafeChild(flags.output, safeFilename(`${_.get(result, 'platform.target_id')}.json`)), JSON.stringify(result, null, 2), ); }); diff --git a/src/commands/convert/prowler2hdf.ts b/src/commands/convert/prowler2hdf.ts index 0b899783a0..ee4eed1158 100644 --- a/src/commands/convert/prowler2hdf.ts +++ b/src/commands/convert/prowler2hdf.ts @@ -1,9 +1,8 @@ import { Flags } from '@oclif/core'; import fs from 'fs'; import { ASFFResults as Mapper, INPUT_TYPES } from '@mitre/hdf-converters'; -import { basename, checkInput, checkSuffix } from '../../utils/global'; +import { checkInput, checkSuffix, resolveSafeChild, safeFilename } from '../../utils/global'; import _ from 'lodash'; -import path from 'path'; import { BaseCommand } from '../../utils/oclif/base_command'; export default class Prowler2HDF extends BaseCommand { @@ -46,7 +45,7 @@ export default class Prowler2HDF extends BaseCommand { _.forOwn(results, (result, filename) => { fs.writeFileSync( - path.join(flags.output, checkSuffix(basename(filename))), + resolveSafeChild(flags.output, safeFilename(checkSuffix(filename))), JSON.stringify(result, null, 2), ); }); diff --git a/src/commands/convert/snyk2hdf.ts b/src/commands/convert/snyk2hdf.ts index e63799c9d3..0cbc0049bd 100644 --- a/src/commands/convert/snyk2hdf.ts +++ b/src/commands/convert/snyk2hdf.ts @@ -1,9 +1,10 @@ import fs from 'fs'; +import path from 'path'; import { INPUT_TYPES, SnykResults as Mapper } from '@mitre/hdf-converters'; import { Flags } from '@oclif/core'; import _ from 'lodash'; import { BaseCommand } from '../../utils/oclif/base_command'; -import { basename, checkInput, checkSuffix } from '../../utils/global'; +import { basename, checkInput, checkSuffix, resolveSafeChild, safeFilename } from '../../utils/global'; export default class Snyk2HDF extends BaseCommand { static readonly usage @@ -42,9 +43,11 @@ export default class Snyk2HDF extends BaseCommand { const converter = new Mapper(data); const result = converter.toHdf(); if (Array.isArray(result)) { + const outputBase = path.dirname(flags.output); + const outputPrefix = safeFilename(flags.output.replaceAll(/\.json/gi, '')); for (const element of result) { fs.writeFileSync( - `${flags.output.replaceAll(/\.json/gi, '')}-${basename(_.get(element, 'platform.target_id') || '')}.json`, + resolveSafeChild(outputBase, safeFilename(`${outputPrefix}-${basename(_.get(element, 'platform.target_id') || '')}.json`)), JSON.stringify(element, null, 2), ); } diff --git a/src/commands/convert/splunk2hdf.ts b/src/commands/convert/splunk2hdf.ts index 706de392b6..604daa3bbf 100644 --- a/src/commands/convert/splunk2hdf.ts +++ b/src/commands/convert/splunk2hdf.ts @@ -3,8 +3,7 @@ import { SplunkMapper } from '@mitre/hdf-converters'; import { table } from 'table'; import _ from 'lodash'; import fs from 'fs'; -import path from 'path'; -import { basename } from '../../utils/global'; +import { resolveSafeChild, safeFilename } from '../../utils/global'; import { createWinstonLogger } from '../../utils/logging'; import { BaseCommand } from '../../utils/oclif/base_command'; @@ -122,9 +121,9 @@ export default class Splunk2HDF extends BaseCommand { const hdf = await mapper.toHdf(input); // Rename example.json -> example-p9dwG2kdSoHsYdyF2dMytUmljgOHD5.json and put into the outputFolder fs.writeFileSync( - path.join( + resolveSafeChild( outputFolder, - basename( + safeFilename( _.get(hdf, 'meta.filename', '').replace(/\.json$/, '') + _.get(hdf, 'meta.guid') + '.json', @@ -138,9 +137,9 @@ export default class Splunk2HDF extends BaseCommand { for (const execution of executions) { const hdf = await mapper.toHdf(_.get(execution, 'meta.guid')); fs.writeFileSync( - path.join( + resolveSafeChild( outputFolder, - basename( + safeFilename( _.get(hdf, 'meta.filename', '').replace(/\.json$/, '') + _.get(hdf, 'meta.guid') + '.json', diff --git a/src/commands/convert/trivy2hdf.ts b/src/commands/convert/trivy2hdf.ts index 0e2e39abb2..4ad469b3c8 100644 --- a/src/commands/convert/trivy2hdf.ts +++ b/src/commands/convert/trivy2hdf.ts @@ -1,9 +1,8 @@ import { Flags } from '@oclif/core'; import fs from 'fs'; import { ASFFResults as Mapper, INPUT_TYPES } from '@mitre/hdf-converters'; -import { basename, checkInput, checkSuffix } from '../../utils/global'; +import { checkInput, checkSuffix, resolveSafeChild, safeFilename } from '../../utils/global'; import _ from 'lodash'; -import path from 'path'; import { BaseCommand } from '../../utils/oclif/base_command'; export default class Trivy2HDF extends BaseCommand { @@ -53,7 +52,7 @@ export default class Trivy2HDF extends BaseCommand { _.forOwn(results, (result, filename) => { fs.writeFileSync( - path.join(flags.output, checkSuffix(basename(filename))), + resolveSafeChild(flags.output, safeFilename(checkSuffix(filename))), JSON.stringify(result, null, 2), ); }); diff --git a/src/commands/generate/ckl_metadata.ts b/src/commands/generate/ckl_metadata.ts index ac54e55c5a..38370817c4 100644 --- a/src/commands/generate/ckl_metadata.ts +++ b/src/commands/generate/ckl_metadata.ts @@ -13,7 +13,7 @@ import { Flags } from '@oclif/core'; import { colorize } from 'json-colorizer'; import { isEmpty } from 'lodash'; import type { Logger } from 'winston'; -import { getJsonMetaDataExamples } from '../../utils/global'; +import { getJsonMetaDataExamples, resolveSafeChild, safeFilename } from '../../utils/global'; import { createWinstonLogger } from '../../utils/logging'; import { BaseCommand } from '../../utils/oclif/base_command'; @@ -58,9 +58,9 @@ export default class GenerateCKLMetadata extends BaseCommand { try { this.logThis(` Generating the summary file on directory: ${shortControlsDir}`, 'info'); // Generate the profile controls summary from the `controlsDir` without the trailing "controls" directory - const inspecJsonFile = execFileSync(inspecPath, ['json', path.dirname(controlsDir)], { encoding: 'utf8', maxBuffer: 50 * 1024 * 1024, shell: process.platform === 'win32' }); + const inspecJsonFile = safeExecFileSync(inspecPath, ['json', path.dirname(controlsDir)], { encoding: 'utf8', maxBuffer: 50 * 1024 * 1024 }); this.logThis(' Generated InSpec Profiles from InSpec JSON summary', 'info'); existingProfile = processInSpecProfile(inspecJsonFile); } catch (error: unknown) { @@ -373,11 +372,12 @@ export default class GenerateDelta extends BaseCommand { // directory with the proper name and Id, than regenerate json profile summary. for (const [key, value] of Object.entries(controls)) { - const sourceShortControlFile = path.join(shortProfileDir, `${value}.rb`); - const mappedShortControlFile = path.join(shortMappedDir, `${value}.rb`); + const controlFilename = safeFilename(`${basename(value)}.rb`); + const sourceShortControlFile = path.join(shortProfileDir, controlFilename); + const mappedShortControlFile = path.join(shortMappedDir, controlFilename); - const sourceControlFile = path.join(controlsDir, `${value}.rb`); - const mappedControlFile = path.join(mappedDir, `${value}.rb`); + const sourceControlFile = resolveSafeChild(controlsDir, controlFilename); + const mappedControlFile = resolveSafeChild(mappedDir, controlFilename); this.logger.info(`${'Mapping (From --> To): '} ${`${value} --> ${key}`}`); @@ -428,7 +428,7 @@ export default class GenerateDelta extends BaseCommand { // Get the directory name without the trailing "controls" directory // Here we are using the newly updated (mapped) controls // const profileDir = path.dirname(controlsDir) - const inspecJsonFileNew = execFileSync(inspecPath, ['json', path.dirname(mappedDir)], { encoding: 'utf8', maxBuffer: 50 * 1024 * 1024, shell: process.platform === 'win32' }); + const inspecJsonFileNew = safeExecFileSync(inspecPath, ['json', path.dirname(mappedDir)], { encoding: 'utf8', maxBuffer: 50 * 1024 * 1024 }); // Replace existing profile (inputted JSON of source profile to be mapped) // Allow delta to take care of the rest @@ -486,14 +486,14 @@ export default class GenerateDelta extends BaseCommand { if (reportFile) { if (fs.existsSync(reportFile) && fs.lstatSync(reportFile).isDirectory()) { // Not a file - directory provided - markDownFile = path.join(reportFile, 'delta.md'); + markDownFile = resolveSafeChild(reportFile, 'delta.md'); } else if (fs.existsSync(reportFile) && fs.lstatSync(reportFile).isFile()) { // File name provided and exists - will be overwritten markDownFile = reportFile; } else if (path.extname(reportFile) === '.md') { markDownFile = reportFile; } else { - markDownFile = path.join(outputProfileFolderPath, 'delta.md'); + markDownFile = resolveSafeChild(outputProfileFolderPath, 'delta.md'); } } else { this.logThis(' An output markdown reports was not requested', 'debug'); @@ -551,17 +551,17 @@ export default class GenerateDelta extends BaseCommand { // method from inspect-objects const newControl = updateControl(existingProfile.controls[index], control, thisLogger); this.logThis(`Writing updated control with code block for: ${control.id}.`, 'info'); - // `basename(control.id)` strips path separators; `resolveSafeChild` - // rejects symlink traversal on the `controls/` subdirectory. + const controlFilename = safeFilename(`${basename(control.id)}.rb`); fs.writeFileSync( - resolveSafeChild(outputProfileFolderPath, 'controls', `${basename(control.id)}.rb`), + resolveSafeChild(outputProfileFolderPath, 'controls', controlFilename), newControl.toRuby(processLogLevel), ); } else { // We didn't find a mapping for this control - Old style of updating controls this.logThis(`Writing new control without code block for: ${control.id}.`, 'info'); + const controlFilename = safeFilename(`${basename(control.id)}.rb`); fs.writeFileSync( - resolveSafeChild(outputProfileFolderPath, 'controls', `${basename(control.id)}.rb`), + resolveSafeChild(outputProfileFolderPath, 'controls', controlFilename), control.toRuby(processLogLevel), ); } @@ -974,7 +974,10 @@ export default class GenerateDelta extends BaseCommand { fs.mkdirSync(mappedDir, { recursive: true }); // Copy the profile inspec.yml to the mapped directory to generate the profile controls summary properly - copyFileSync(path.join(destFilePath, 'inspec.yml'), path.join(path.dirname(mappedDir), 'inspec.yml')); + copyFileSync( + resolveSafeChild(destFilePath, 'inspec.yml'), + resolveSafeChild(path.dirname(mappedDir), 'inspec.yml'), + ); return mappedDir; } catch (error: unknown) { diff --git a/src/commands/generate/inspec_profile.ts b/src/commands/generate/inspec_profile.ts index e7e8761ac5..7665d5c47f 100644 --- a/src/commands/generate/inspec_profile.ts +++ b/src/commands/generate/inspec_profile.ts @@ -6,7 +6,7 @@ import { XMLParser } from 'fast-xml-parser'; import _ from 'lodash'; import type { Logger } from 'winston'; import type { InSpecMetaData, InspecReadme } from '../../types/inspec'; -import { basename } from '../../utils/global'; +import { basename, resolveSafeChild, safeFilename } from '../../utils/global'; import { createWinstonLogger } from '../../utils/logging'; import { BaseCommand } from '../../utils/oclif/base_command'; @@ -118,7 +118,7 @@ export default class InspecProfile extends BaseCommand { const benchmarkTitle = isSTIG ? _.get(xmlDoc, 'Benchmark.title') : _.get(xmlDoc, 'xccdf:Benchmark.xccdf:title.#text'); outDir = (benchmarkTitle === undefined) ? flags.output - : basename(benchmarkTitle.replace('Security Technical Implementation Guide', 'stig-baseline').replaceAll(' ', '-').toLowerCase()); + : safeFilename(benchmarkTitle.replace('Security Technical Implementation Guide', 'stig-baseline').replaceAll(' ', '-').toLowerCase()); } else { outDir = flags.output; } @@ -224,15 +224,15 @@ export default class InspecProfile extends BaseCommand { .join('\n\n'); logger.debug(`Writing control to: ${path.join(outDir, 'controls', 'controls.rb')}`); fs.writeFileSync( - path.join(outDir, 'controls', 'controls.rb'), + resolveSafeChild(outDir, 'controls', 'controls.rb'), controls, ); } else { for (const control of profile.controls) { - const controlId = basename(control.id); // Ensure valid filename - logger.debug(`Writing control to: ${path.join(outDir, 'controls', controlId + '.rb')}`); + const controlFilename = safeFilename(`${basename(control.id)}.rb`); + logger.debug(`Writing control to: ${path.join(outDir, 'controls', controlFilename)}`); fs.writeFileSync( - path.join(outDir, 'controls', controlId + '.rb'), + resolveSafeChild(outDir, 'controls', controlFilename), control.toRuby(), ); } @@ -618,7 +618,7 @@ ${contentObj.profileType === 'CIS' : '[DISA STIGs are published by DISA IASE](https://public.cyber.mil/stigs/)' } `; - fs.writeFile(path.join(outDir, 'README.md'), readmeContent, (err) => { + fs.writeFile(resolveSafeChild(outDir, 'README.md'), readmeContent, (err) => { if (err) { logger.error(`Error saving the README.md file to: ${outDir}. Cause: ${err}`); } else { @@ -650,7 +650,7 @@ function generateYaml(profile: Profile, outDir: string, logger: Logger) { inputs: `; - fs.writeFile(path.join(outDir, 'inspec.yml'), inspecYmlContent, (err) => { + fs.writeFile(resolveSafeChild(outDir, 'inspec.yml'), inspecYmlContent, (err) => { if (err) { logger.error(`Error saving the inspec.yml file to: ${outDir}. Cause: ${err}`); } else { @@ -675,7 +675,7 @@ are permitted provided that the following conditions are met: used to endorse or promote products derived from this software without specific prior written permission. `; - fs.writeFile(path.join(outDir, 'LICENSE.md'), licensesContent, (err) => { + fs.writeFile(resolveSafeChild(outDir, 'LICENSE.md'), licensesContent, (err) => { if (err) { logger.error(`Error saving the LICENSE file to: ${outDir}. Cause: ${err}`); } else { @@ -699,7 +699,7 @@ express written permission of The MITRE Corporation. For further information, please contact The MITRE Corporation, Contracts Management Office, 7515 Colshire Drive, McLean, VA 22102-7539, (703) 983-6000. `; - fs.writeFile(path.join(outDir, 'NOTICE.md'), noticeContent, (err) => { + fs.writeFile(resolveSafeChild(outDir, 'NOTICE.md'), noticeContent, (err) => { if (err) { logger.error(`Error saving the NOTICE.md file to: ${outDir}. Cause: ${err}`); } else { @@ -879,7 +879,7 @@ Style/StringChars: # new in 1.12 Style/SwapValues: # new in 1.1 Enabled: true `; - fs.writeFile(path.join(outDir, '.rubocop.yml'), robocopContent, (err) => { + fs.writeFile(resolveSafeChild(outDir, '.rubocop.yml'), robocopContent, (err) => { if (err) { logger.error(`Error saving the .rubocop.yml file to: ${outDir}. Cause: ${err}`); } else { @@ -892,7 +892,7 @@ function generateGemRc(outDir: string, logger: Logger) { const gemRc = `gem: --no-document `; - fs.writeFile(path.join(outDir, '.gemrc'), gemRc, (err) => { + fs.writeFile(resolveSafeChild(outDir, '.gemrc'), gemRc, (err) => { if (err) { logger.error(`Error saving the .gemrc file to: ${outDir}. Cause: ${err}`); } else { @@ -930,7 +930,7 @@ source 'https://rubygems.cinc.sh/' do gem 'inspec-core' end `; - fs.writeFile(path.join(outDir, 'Gemfile'), gemFileContent, (err) => { + fs.writeFile(resolveSafeChild(outDir, 'Gemfile'), gemFileContent, (err) => { if (err) { logger.error(`Error saving the Gemfile file to: ${outDir}. Cause: ${err}`); } else { @@ -966,7 +966,7 @@ end desc 'pre-commit checks' task pre_commit_checks: [:lint, 'inspec:check'] `; - fs.writeFile(path.join(outDir, 'Rakefile'), rakefileContent, (err) => { + fs.writeFile(resolveSafeChild(outDir, 'Rakefile'), rakefileContent, (err) => { if (err) { logger.error(`Error saving the Rakefile file to: ${outDir}. Cause: ${err}`); } else { @@ -1053,7 +1053,7 @@ report.md check-results.txt kitchen.local.ec2.yml `; - fs.writeFile(path.join(outDir, '.gitignore'), gitignoreContent, (err) => { + fs.writeFile(resolveSafeChild(outDir, '.gitignore'), gitignoreContent, (err) => { if (err) { logger.error(`Error saving the .gitignore file to: ${outDir}. Cause: ${err}`); } else { diff --git a/src/commands/generate/spreadsheet2inspec_stub.ts b/src/commands/generate/spreadsheet2inspec_stub.ts index 305197433f..a41019a3ba 100644 --- a/src/commands/generate/spreadsheet2inspec_stub.ts +++ b/src/commands/generate/spreadsheet2inspec_stub.ts @@ -10,7 +10,7 @@ import CISNistMappings from '../../resources/cis2nist.json'; import files from '../../resources/files.json'; import type { CSVControl } from '../../types/csv'; import type { InSpecControl, InSpecMetaData } from '../../types/inspec'; -import { basename, extractValueViaPathOrNumber } from '../../utils/global'; +import { basename, extractValueViaPathOrNumber, resolveSafeChild, safeFilename } from '../../utils/global'; import { BaseCommand } from '../../utils/oclif/base_command'; import { impactNumberToSeverityString, inspecControlToRubyCode, severityStringToImpact } from '../../utils/xccdf2inspec'; @@ -180,7 +180,7 @@ export default class Spreadsheet2HDF extends BaseCommand version: metadata.version || '0.1.0', }; - fs.writeFileSync(path.join(flags.output, 'inspec.yml'), YAML.stringify(profileInfo)); + fs.writeFileSync(resolveSafeChild(flags.output, 'inspec.yml'), YAML.stringify(profileInfo)); // Write README.md const readableMetadata: Record = {}; @@ -190,7 +190,7 @@ export default class Spreadsheet2HDF extends BaseCommand readableMetadata[_.startCase(key)] = value; } } - fs.writeFileSync(path.join(flags.output, 'README.md'), `# ${profileInfo.name}\n${profileInfo.summary}\n---\n${YAML.stringify(readableMetadata)}`); + fs.writeFileSync(resolveSafeChild(flags.output, 'README.md'), `# ${profileInfo.name}\n${profileInfo.summary}\n---\n${YAML.stringify(readableMetadata)}`); try { const workBook = await XlsxPopulate.fromFileAsync(flags.input); @@ -325,7 +325,7 @@ export default class Spreadsheet2HDF extends BaseCommand // Convert all extracted controls to Ruby/InSpec code for (const control of inspecControls) { - fs.writeFileSync(path.join(flags.output, 'controls', basename(control.id) + '.rb'), inspecControlToRubyCode(control, flags.lineLength, flags.encodingHeader)); + fs.writeFileSync(resolveSafeChild(flags.output, 'controls', safeFilename(`${basename(control.id)}.rb`)), inspecControlToRubyCode(control, flags.lineLength, flags.encodingHeader)); } } } diff --git a/src/commands/generate/update_controls4delta.ts b/src/commands/generate/update_controls4delta.ts index 15df744864..80b5b131dd 100644 --- a/src/commands/generate/update_controls4delta.ts +++ b/src/commands/generate/update_controls4delta.ts @@ -1,4 +1,3 @@ -import { execFileSync } from 'child_process'; import fs from 'fs'; import { readdir } from 'fs/promises'; import path from 'path'; @@ -11,7 +10,7 @@ import { import { Flags } from '@oclif/core'; import tmp from 'tmp'; import type { Logger } from 'winston'; -import { basename, downloadFile, extractFileFromZip, getErrorMessage } from '../../utils/global'; +import { basename, downloadFile, extractFileFromZip, getErrorMessage, resolveSafeChild, safeExecFileSync, safeFilename } from '../../utils/global'; import { createWinstonLogger } from '../../utils/logging'; import { BaseCommand } from '../../utils/oclif/base_command'; @@ -264,7 +263,7 @@ export default class GenerateUpdateControls extends BaseCommand[\]{}^=;!'+,`~()@"%\t\r\n]/; + for (const value of [command, ...args]) { + if (unsafeCharacters.test(value)) { + throw new Error('Unsafe cmd.exe shell characters detected; refusing to run command through the Windows shell.'); + } + } + } + + return execFileSync(command, args, { ...options, shell: process.platform === 'win32' }); } /** diff --git a/test/commands/generate/update_controls4delta.test.ts b/test/commands/generate/update_controls4delta.test.ts index c5b894def5..674357f8a2 100644 --- a/test/commands/generate/update_controls4delta.test.ts +++ b/test/commands/generate/update_controls4delta.test.ts @@ -4,12 +4,21 @@ import path from 'path'; import tmp from 'tmp'; import { describe, expect, it } from 'vitest'; +const itOnWindows = process.platform === 'win32' ? it : it.skip; + +function createTempWorkspace() { + return tmp.dirSync({ + unsafeCleanup: true, + ...(process.platform === 'win32' ? { tmpdir: process.cwd() } : {}), + }); +} + // Functional tests describe('Test generate update_controls4delta command', () => { // This command updates controls in place, so tests must operate on a temp copy // of the fixture controls rather than the checked-in sample files it('should rename legacy controls to the new XCCDF control ids using --inspecJsonFile', async () => { - const tempWorkspace = tmp.dirSync({ unsafeCleanup: true }); + const tempWorkspace = createTempWorkspace(); const sourceControlsDir = path.resolve('./test/sample_data/inspec/json/profile_and_controls/windows_server_2019_v1r3_mini_controls'); const tempControlsDir = path.join(tempWorkspace.name, 'controls'); @@ -46,13 +55,13 @@ describe('Test generate update_controls4delta command', () => { }); it('should rename legacy controls to the new XCCDF control ids using --inspecPath', async () => { - const tempWorkspace = tmp.dirSync({ unsafeCleanup: true }); + const tempWorkspace = createTempWorkspace(); const sourceControlsDir = path.resolve('./test/sample_data/inspec/json/profile_and_controls/windows_server_2019_v1r3_mini_controls'); const tempControlsDir = path.join(tempWorkspace.name, 'controls'); fs.cpSync(sourceControlsDir, tempControlsDir, { recursive: true }); - await runCommand<{ name: string }>([ + const { error, stderr } = await runCommand<{ name: string }>([ 'generate update_controls4delta', '-I', 'cinc-auditor', '-X', path.resolve('./test/sample_data/xccdf/stigs/Windows_Server_2019_V3R2_xccdf.xml'), @@ -60,6 +69,9 @@ describe('Test generate update_controls4delta command', () => { '--no-backupControls', ]); + expect(error, stderr).toBeUndefined(); + expect(stderr).toBe(''); + const fileCount = fs.readdirSync(tempControlsDir).length; expect(fileCount).to.eql(5); @@ -75,4 +87,24 @@ describe('Test generate update_controls4delta command', () => { expect(fs.existsSync(path.join(tempControlsDir, 'V-93473.rb'))).to.eql(false); expect(fs.existsSync(path.join(tempControlsDir, 'V-93461.rb'))).to.eql(false); }); + + itOnWindows('should report Windows shell metacharacters before generating an inspec summary with --inspecPath', async () => { + const tempWorkspace = createTempWorkspace(); + const sourceControlsDir = path.resolve('./test/sample_data/inspec/json/profile_and_controls/windows_server_2019_v1r3_mini_controls'); + const profileDir = path.join(tempWorkspace.name, 'profile&ver'); + const tempControlsDir = path.join(profileDir, 'controls'); + + fs.mkdirSync(profileDir); + fs.cpSync(sourceControlsDir, tempControlsDir, { recursive: true }); + + const { stderr } = await runCommand<{ name: string }>([ + 'generate update_controls4delta', + '-I', 'cinc-auditor', + '-X', path.resolve('./test/sample_data/xccdf/stigs/Windows_Server_2019_V3R2_xccdf.xml'), + '-c', tempControlsDir, + '--no-backupControls', + ]); + + expect(stderr).to.match(/Unsafe cmd\.exe shell characters/); + }); }); diff --git a/test/commands/view/heimdall_view_cli.test.ts b/test/commands/view/heimdall_view_cli.test.ts index 088aa12cec..557a1bcfc1 100644 --- a/test/commands/view/heimdall_view_cli.test.ts +++ b/test/commands/view/heimdall_view_cli.test.ts @@ -1,6 +1,6 @@ -import type { AddressInfo } from 'node:net'; -import type { Server } from 'node:http'; -import path from 'node:path'; +import type { AddressInfo } from 'net'; +import type { Server } from 'http'; +import path from 'path'; import { runCommand } from '@oclif/test'; import axios from 'axios'; import express from 'express'; diff --git a/test/utils/__tests__/cross_vendor_integration.test.ts b/test/utils/__tests__/cross_vendor_integration.test.ts index ddaa4fc1ab..20e228bbb0 100644 --- a/test/utils/__tests__/cross_vendor_integration.test.ts +++ b/test/utils/__tests__/cross_vendor_integration.test.ts @@ -1,5 +1,5 @@ -import fs from 'node:fs'; -import path from 'node:path'; +import fs from 'fs'; +import path from 'path'; import { describe, expect, it } from 'vitest'; import { applyRequirementFirstPipeline, diff --git a/test/utils/__tests__/global.test.ts b/test/utils/__tests__/global.test.ts index b3af848c40..74f7e08b31 100644 --- a/test/utils/__tests__/global.test.ts +++ b/test/utils/__tests__/global.test.ts @@ -1,8 +1,8 @@ -import fs from 'node:fs'; -import os from 'node:os'; -import path from 'node:path'; +import fs from 'fs'; +import os from 'os'; +import path from 'path'; import type { ExecJSON, ContextualizedEvaluation } from 'inspecjs'; -import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; +import { afterAll, afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; import { basename, checkSuffix, @@ -11,6 +11,7 @@ import { dataURLtoU8Array, getDescription, arrayNeededPaths, resolveSafeChild, + safeFilename, } from '../../../src/utils/global'; const UTF8_ENCODING = 'utf8'; @@ -60,9 +61,12 @@ describe('basename', () => { expect(result).toBe('file.txt'); }); - it('should return the last directory name if the path ends with a forward slash (non windows)', () => { - const result = basename('/path/to/'); - expect(result).toBe('to'); + it.each([ + ['/path/to/', 'forward slash'], + ['\\path\\to\\', 'backslash'], + ['\\path\\to\\\\\\', 'multiple backslashes'], + ])('should return the last directory name if the path ends with a %s', (inputPath) => { + expect(basename(inputPath)).toBe('to'); }); it('should handle paths with no slashes', () => { @@ -75,9 +79,9 @@ describe('basename', () => { expect(result).toBe('file.txt'); }); - it('should return the last directory name if the path ends with a backslash\'s (windows)', () => { - const result = basename('\\path\\to\\'); - expect(result).toBe('to'); + it('should trim trailing whitespace before returning the filename', () => { + const result = basename('file.txt '); + expect(result).toBe('file.txt'); }); it('should return the empty string if the path is the empty string', () => { @@ -85,12 +89,111 @@ describe('basename', () => { expect(result).toBe(''); }); + it('should return the empty string if trimEnd leaves an empty string', () => { + const result = basename(' '); + expect(result).toBe(''); + }); + it('should return the empty string if the path consists only of separators', () => { const result = basename(String.raw`//////\\\/\/`); expect(result).toBe(''); }); }); +describe('safeFilename', () => { + it('should return the basename when the filename is safe', () => { + const result = safeFilename('/path/to/file.txt'); + expect(result).toBe('file.txt'); + }); + + it('should throw when the basename is empty', () => { + expect(() => safeFilename(' ')).toThrow(/Unsafe filename/); + }); + + it('should throw when the filename contains reserved characters', () => { + expect(() => safeFilename('bad:name.txt')).toThrow(/Unsafe filename/); + }); + + it('should throw when the filename contains control characters', () => { + expect(() => safeFilename('bad\u0000name.txt')).toThrow(/Unsafe filename/); + }); + + it('should throw when the filename is a Windows reserved name', () => { + expect(() => safeFilename('CON.txt')).toThrow(/Unsafe filename/); + }); +}); + +describe.sequential('safeExecFileSync', () => { + const execFileSyncMock = vi.fn(); + + async function importGlobalWithPlatform(platform: NodeJS.Platform) { + vi.resetModules(); + execFileSyncMock.mockReset(); + execFileSyncMock.mockReturnValue('stdout'); + + vi.doMock('child_process', async () => { + const actual = await vi.importActual('child_process'); + return { ...actual, execFileSync: execFileSyncMock }; + }); + + vi.doMock('process', async () => { + const actual = await vi.importActual('process'); + return { + ...actual, + default: Object.create(actual.default, { + platform: { value: platform, enumerable: true }, + }), + }; + }); + + return import('../../../src/utils/global'); + } + + afterEach(() => { + vi.doUnmock('child_process'); + vi.doUnmock('process'); + vi.resetModules(); + }); + + it('calls execFileSync without a shell on non-Windows platforms', async () => { + const { safeExecFileSync } = await importGlobalWithPlatform('linux'); + + const result = safeExecFileSync('inspec', ['json', '/tmp/profile'], { encoding: 'utf8' }); + + expect(result).toBe('stdout'); + expect(execFileSyncMock).toHaveBeenCalledWith('inspec', ['json', '/tmp/profile'], { + encoding: 'utf8', + shell: false, + }); + }); + + it('uses the Windows shell when arguments do not contain cmd.exe metacharacters', async () => { + const { safeExecFileSync } = await importGlobalWithPlatform('win32'); + + const result = safeExecFileSync(String.raw`C:\Program Files\InSpec\inspec.bat`, [ + 'json', + String.raw`C:\tmp\profile\control $psOnly#chars`, + ], { encoding: 'utf8', maxBuffer: 1024 }); + + expect(result).toBe('stdout'); + expect(execFileSyncMock).toHaveBeenCalledWith(String.raw`C:\Program Files\InSpec\inspec.bat`, [ + 'json', + String.raw`C:\tmp\profile\control $psOnly#chars`, + ], { encoding: 'utf8', maxBuffer: 1024, shell: true }); + }); + + it('rejects cmd.exe metacharacters and BatBadBut argument parsing cases on Windows', async () => { + const { safeExecFileSync } = await importGlobalWithPlatform('win32'); + + for (const unsafeCharacter of ['&', '|', '<', '>', '[', ']', '{', '}', '^', '=', ';', '!', '\'', '+', ',', '`', '~', '(', ')', '@', '"', '%', '\t', '\r', '\n']) { + expect(() => safeExecFileSync('inspec', ['json', `profile${unsafeCharacter}name`], { encoding: 'utf8' })) + .toThrow(/Unsafe cmd\.exe shell characters/); + } + + expect(execFileSyncMock).not.toHaveBeenCalled(); + }); +}); + describe('dataURLtoU8Array', () => { it('should convert a data URL to a Uint8Array', () => { const dataURL = 'data:text/plain;base64,SGVsbG8sIFdvcmxkIQ=='; // "Hello, World!" in base64 @@ -396,54 +499,69 @@ describe.sequential('checkInput', () => { }); describe('resolveSafeChild', () => { - let tmpBase = ''; + const tempDirs: string[] = []; - beforeEach(() => { - tmpBase = fs.mkdtempSync(path.join(os.tmpdir(), 'saf-safe-child-')); - }); + async function makeTempDir(prefix: string): Promise { + const dir = await fs.promises.mkdtemp(path.join(os.tmpdir(), prefix)); + tempDirs.push(dir); + return dir; + } - afterEach(() => { - if (tmpBase) { - fs.rmSync(tmpBase, { recursive: true, force: true }); - } + afterAll(async () => { + await Promise.all( + tempDirs.map(dir => fs.promises.rm(dir, { recursive: true, force: true })), + ); }); - it('resolves a simple child path under the base directory', () => { + it('resolves a simple child path under the base directory', async () => { + const tmpBase = await makeTempDir('saf-safe-child-'); const resolved = resolveSafeChild(tmpBase, 'delta.json'); expect(resolved).toBe(path.join(fs.realpathSync(tmpBase), 'delta.json')); }); - it('resolves a nested child when intermediate directories are normal', () => { + it('resolves a nested child when intermediate directories are normal', async () => { + const tmpBase = await makeTempDir('saf-safe-child-'); fs.mkdirSync(path.join(tmpBase, 'controls')); const resolved = resolveSafeChild(tmpBase, 'controls', 'SV-1.rb'); expect(resolved).toBe(path.join(fs.realpathSync(tmpBase), 'controls', 'SV-1.rb')); }); - it('throws on `..` escape attempts', () => { + it('throws on `..` escape attempts', async () => { + const tmpBase = await makeTempDir('saf-safe-child-'); expect(() => resolveSafeChild(tmpBase, '..', 'escape.txt')).toThrow( /outside output directory/, ); }); - it('throws when an intermediate directory is a symlink', () => { - const outside = fs.mkdtempSync(path.join(os.tmpdir(), 'saf-outside-')); + it('throws when an intermediate directory is a symlink', async () => { + const tmpBase = await makeTempDir('saf-safe-child-'); + const outside = await makeTempDir('saf-outside-'); const linkPath = path.join(tmpBase, 'controls'); - // Defensive: beforeEach gives a fresh tmpBase, but some vitest test-file - // orderings appear to surface EEXIST on symlinkSync. Scrub first. - if (fs.existsSync(linkPath)) { - fs.rmSync(linkPath, { recursive: true, force: true }); - } - try { - fs.symlinkSync(outside, linkPath); - expect(() => resolveSafeChild(tmpBase, 'controls', 'SV-1.rb')).toThrow( - /symlink/, - ); - } finally { - fs.rmSync(outside, { recursive: true, force: true }); - } + fs.symlinkSync(outside, linkPath); + + expect(() => resolveSafeChild(tmpBase, 'controls', 'SV-1.rb')).toThrow( + /symlink/, + ); + }); + + it('throws when the target file is a symlink', async () => { + const tmpBase = await makeTempDir('saf-safe-child-'); + const outside = await makeTempDir('saf-outside-'); + const outsideFile = path.join(outside, 'target.rb'); + const controlsDir = path.join(tmpBase, 'controls'); + const linkPath = path.join(controlsDir, 'SV-1.rb'); + + fs.writeFileSync(outsideFile, 'outside'); + fs.mkdirSync(controlsDir); + fs.symlinkSync(outsideFile, linkPath); + + expect(() => resolveSafeChild(tmpBase, 'controls', 'SV-1.rb')).toThrow( + /symlink/, + ); }); - it('throws when baseDir does not exist', () => { + it('throws when baseDir does not exist', async () => { + const tmpBase = await makeTempDir('saf-safe-child-'); const nonexistent = path.join(tmpBase, 'does-not-exist'); expect(() => resolveSafeChild(nonexistent, 'x.txt')).toThrow(); }); diff --git a/test/utils/__tests__/logging.test.ts b/test/utils/__tests__/logging.test.ts index 86df3c7de6..90ac6d38e8 100644 --- a/test/utils/__tests__/logging.test.ts +++ b/test/utils/__tests__/logging.test.ts @@ -1,7 +1,7 @@ -import { randomBytes } from 'node:crypto'; -import fs from 'node:fs'; -import os from 'node:os'; -import path from 'node:path'; +import { randomBytes } from 'crypto'; +import fs from 'fs'; +import os from 'os'; +import path from 'path'; import { describe, expect, it, vi } from 'vitest'; import * as winston from 'winston'; import type { Logger } from 'winston';