Fix security issue: Path traversal in JavaScript

libs/screenshot/compare.mjs

@@ -1,13 +1,39 @@
 // eslint-disable-next-line import/no-extraneous-dependencies, import/extensions
 import { getComparator } from 'playwright-core/lib/utils';
-import fs from 'fs';
 import axios from 'axios';
 import path from 'path';
+import fs from 'fs';
 
 const S3URL = 'https://s3-sj3.corp.adobe.com/milo';
+const ALLOWED_BASE_DIRECTORY = 'screenshots/';
+
+function sanitizeAndValidateFilePath(filePath) {
+  if (typeof filePath !== 'string') {
+    throw new Error(`Invalid path: ${filePath}. Path should be a string.`);
+  }
+
+  // Resolve the input path to an absolute path
+  const absolutePath = path.resolve(filePath);
+  console.log(absolutePath);
+
+  // Ensure the path is within the allowed base directory
+  if (!absolutePath.includes(ALLOWED_BASE_DIRECTORY)) {
+    throw new Error(`Path traversal attempt detected: ${filePath}`);
+  }
+
+  if (!fs.existsSync(absolutePath)) {
+    throw new Error(`File does not exist: ${absolutePath}`);
+  }
+
+  if (!fs.lstatSync(absolutePath).isFile()) {
+    throw new Error(`Not a file: ${absolutePath}`);
+  }
+
+  return absolutePath;
+}
 
 async function downloadImage(url, localPath) {
-  const writer = fs.createWriteStream(localPath);
+  const writer = fs.createWriteStream(sanitizeAndValidateFilePath(localPath));
 
   const res = await axios.get(url, { responseType: 'stream' });
 
@@ -58,7 +84,7 @@ async function main() {
     process.exit(1);
   }
 
-  const curEntries = JSON.parse(fs.readFileSync(`${localPath}/results.json`));
+  const curEntries = JSON.parse(fs.readFileSync(sanitizeAndValidateFilePath(`${localPath}/results.json`)));
 
   const firstEntry = Object.values(curEntries)[0][0];
 
@@ -80,8 +106,8 @@ async function main() {
       const result = {};
       console.log(entry);
 
-      const baseImage = fs.readFileSync(entry.a);
-      const currImage = fs.readFileSync(entry.b);
+      const baseImage = fs.readFileSync(sanitizeAndValidateFilePath(entry.a));
+      const currImage = fs.readFileSync(sanitizeAndValidateFilePath(entry.b));
       result.order = entry.order;
       result.a = entry.a;
       result.b = entry.b;
@@ -101,7 +127,7 @@ async function main() {
     results[key] = resultsArray;
   }
 
-  fs.writeFileSync(`${localPath}/results.json`, JSON.stringify(results, null, 2));
+  fs.writeFileSync(sanitizeAndValidateFilePath(`${localPath}/results.json`), JSON.stringify(results, null, 2));
 }
 
 main();

libs/screenshot/merge.js

@@ -1,19 +1,24 @@
 const fs = require('fs');
 const path = require('path');
+const { validatePath } = require('./utils.js');
 
 function mergeResults(folderPath) {
   try {
-    const resultsFiles = fs.readdirSync(folderPath).filter((file) => file.startsWith('results-'));
+    const resultsFiles = fs.readdirSync(validatePath(folderPath, { allowDirectory: true }))
+      .filter((file) => file.startsWith('results-'));
     let finalResults = {};
 
     // eslint-disable-next-line no-restricted-syntax
     for (const file of resultsFiles) {
-      const content = JSON.parse(fs.readFileSync(path.join(folderPath, file), 'utf-8'));
+      const content = JSON.parse(fs.readFileSync(validatePath(path.join(folderPath, file)), 'utf-8'));
       finalResults = { ...finalResults, ...content };
     }
 
     // Write the final merged results
-    fs.writeFileSync(`${folderPath}/results.json`, JSON.stringify(finalResults, null, 2));
+    fs.writeFileSync(
+      validatePath(`${folderPath}/results.json`, { forWriting: true }),
+      JSON.stringify(finalResults, null, 2),
+    );
 
     // Optionally, clean up individual files
     resultsFiles.forEach((file) => fs.unlinkSync(path.join(folderPath, file)));

libs/screenshot/uploads3.js

@@ -2,6 +2,7 @@
 const { S3Client, PutObjectCommand } = require('@aws-sdk/client-s3');
 const fs = require('fs');
 const path = require('path');
+const { validatePath } = require('./utils.js');
 
 const S3REGION = 'us-west-1';
 const S3ENDPOINT = 'https://s3-sj3.corp.adobe.com';
@@ -32,7 +33,7 @@ async function uploadFile(fileName, s3Bucket, s3Path, credentials, s3Key, mimeTy
   const baseName = path.basename(fileName);
   const key = path.join(s3Path, s3Key || baseName).replace(/\\/g, '/');
 
-  const fileContent = fs.readFileSync(fileName);
+  const fileContent = fs.readFileSync(validatePath(fileName));
 
   const params = {
     Bucket: s3Bucket,
@@ -56,7 +57,7 @@ async function main() {
   }
 
   const resultsPath = path.join(dir, 'results.json');
-  const entries = JSON.parse(fs.readFileSync(resultsPath));
+  const entries = JSON.parse(fs.readFileSync(validatePath(resultsPath)));
 
   console.log(entries);
 
@@ -110,7 +111,7 @@ async function main() {
   const timestampPath = path.join(dir, 'timestamp.json');
 
   fs.writeFileSync(
-    timestampPath,
+    validatePath(timestampPath, { forWriting: true }),
     JSON.stringify([(new Date()).toLocaleString()], null, 2),
   );
 

libs/screenshot/utils.js

@@ -1,7 +1,50 @@
-// eslint-disable-next-line import/no-extraneous-dependencies, import/extensions, import/no-import-module-exports
-import { getComparator } from 'playwright-core/lib/utils';
+// eslint-disable-next-line import/no-extraneous-dependencies, import/no-import-module-exports, import/extensions
+const { getComparator } = require('playwright-core/lib/utils');
 
 const fs = require('fs');
+const path = require('path');
+
+const ALLOWED_BASE_DIRECTORY = 'screenshots/';
+
+function validatePath(filePath, options = { allowDirectory: false, forWriting: false }) {
+  if (typeof filePath !== 'string') {
+    throw new Error(`Invalid path: ${filePath}. Path should be a string.`);
+  }
+
+  // Resolve the input path to an absolute path
+  const absolutePath = path.resolve(filePath);
+  console.log(absolutePath);
+
+  // Ensure the path is within the allowed base directory
+  if (!absolutePath.startsWith(path.resolve(ALLOWED_BASE_DIRECTORY))) {
+    throw new Error(`Path traversal attempt detected: ${filePath}`);
+  }
+
+  const dirPath = path.dirname(absolutePath);
+
+  if (options.forWriting) {
+    // Ensure the directory exists, or create it if necessary
+    if (!fs.existsSync(dirPath)) {
+      fs.mkdirSync(dirPath, { recursive: true });
+    } else if (!fs.lstatSync(dirPath).isDirectory()) {
+      throw new Error(`Not a directory: ${dirPath}`);
+    }
+  } else {
+    // Check if the path exists
+    if (!fs.existsSync(absolutePath)) {
+      throw new Error(`File or directory does not exist: ${absolutePath}`);
+    }
+
+    const stats = fs.lstatSync(absolutePath);
+    if (options.allowDirectory && !stats.isFile() && !stats.isDirectory()) {
+      throw new Error(`Not a file or directory: ${absolutePath}`);
+    } else if (!options.allowDirectory && !stats.isFile()) {
+      throw new Error(`Not a file: ${absolutePath}`);
+    }
+  }
+
+  return absolutePath;
+}
 
 function compareScreenshots(stableArray, betaArray) {
   const results = [];
@@ -15,13 +58,13 @@ function compareScreenshots(stableArray, betaArray) {
       result.b = `${betaArray[i].a}`;
       urls.push(stableArray[i].urls);
       urls.push(betaArray[i].urls);
-      const stableImage = fs.readFileSync(`${stableArray[i].a}`);
-      const betaImage = fs.readFileSync(`${betaArray[i].a}`);
+      const stableImage = fs.readFileSync(validatePath(`${stableArray[i].a}`));
+      const betaImage = fs.readFileSync(validatePath(`${betaArray[i].a}`));
       const diffImage = comparator(stableImage, betaImage);
 
       if (diffImage) {
         result.diff = `${stableArray[i].a}-diff.png`;
-        fs.writeFileSync(`${stableArray[i].a}-diff.png`, diffImage.diff);
+        fs.writeFileSync(validatePath(`${stableArray[i].a}-diff.png`), diffImage.diff);
         console.info('Differences found');
       }
       result.urls = urls.join(' | ');
@@ -36,7 +79,7 @@ function compareScreenshots(stableArray, betaArray) {
 
 function writeResultsToFile(folderPath, testInfo, results) {
   const resultFilePath = `${folderPath}/results-${testInfo.workerIndex}.json`;
-  fs.writeFileSync(resultFilePath, JSON.stringify(results, null, 2));
+  fs.writeFileSync(validatePath(resultFilePath, { forWriting: true }), JSON.stringify(results, null, 2));
 }
 
-module.exports = { compareScreenshots, writeResultsToFile };
+module.exports = { compareScreenshots, writeResultsToFile, validatePath };