Weak Prefix File Name Generation

When working with file names in Node.js, such as through capabilities you may want to support around file uploads and general file handling, you may theorize that seeding a random set of characters for file name handling is a good security control. It may seem at first, but let’s explore why this is not a good practice.

Weak Prefix File Name Generation

Let’s take the following example of a file upload handler in Node.js that relies on the Express web framework and the core fs module in Node.js to handle file uploads:

const express = require('express');
const fs = require('fs');
const os = require('os');
const path = require('path');

const app = express();
const port = 3000;

app.post('/api/process-pdf', (req, res) => {
  let fileData = [];

  req.on('data', chunk => {
    fileData.push(chunk);
  });

  req.on('end', () => {
    if (fileData.length === 0) {
      return res.status(400).json({ error: 'No file uploaded' });
    }

    const completeFileData = Buffer.concat(fileData);

    // extract file name from the POST body upload of the multipart form data:
    const fileName = req.body.fileName;

    // Create a temporary directory
    const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'pdf-'));
    const filePath = path.join(tmpDir, fileName);

    fs.writeFile(filePath, completeFileData, err => {
      if (err) {
        console.error('Error saving file:', err);
        return res.status(500).json({ error: 'Error saving file' });
      }

      try {
        // *** Your PDF processing logic here.  Use filePath to access the PDF. ***
        console.log('PDF file saved to:', filePath);

        // Replace with your actual data
        const processedData = { message: 'PDF processed successfully' };

        res.json(processedData);

      } catch (processingError) {
        console.error("Error processing PDF:", processingError);
        res.status(500).json({ error: "Error processing PDF" });
      } finally {
        // Delete the temporary file and directory
        fs.unlinkSync(filePath);
        fs.rmdirSync(tmpDir, { recursive: true });
      }

    });
  });
});


app.listen(port, () => {
  console.log(`Server listening at http://localhost:${port}`);
});

From the above, the following line is responsible for the random generation of file paths on disk:

    // extract file name from the POST body upload of the multipart form data:
    const fileName = req.body.fileName;

    // Create a temporary directory
    const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'pdf-'));
    const filePath = path.join(tmpDir, fileName);

The above would result in a path such as:

/var/folders/3s/pyss21dn5m909nm636vx43600000gn/T/pdf-bankstatement.pdf
|
|____________________________________________| |____||________________|
                     |                           |           |
          Temporary Directory               pdf- prefix   File Name

If the fileName contains path traversal characters then the file path will result in an entirely different path location on disk. Imagine the file name is /../../../../../etc/passwd, the resulting file path would be /var/folders/3s/pyss21dn5m909nm636vx43600000gn/T/../../../../../etc/passwd. This would result in the file being written to /etc/passwd on the system.

Such a payload for fileName that begins with a / will bypass the pdf- prefix, and the rest of the ../ path traversal string will bypass the os.tmpdir() path.

Another Weak Prefix Variation

Another variation of the above code is to use a randomly generated string as a prefix and append it to the file name.

For example:

const prefix = Math.random().toString(36).substring(7);
// results in something like this:
// prefix = 'mod8js'

const fileUploadDir = 'uploads/';
const fileName = req.body.fileName;

const filePath = path.join(fileUploadDir, prefix + fileName);

Similarly to the above example, if the fileName contains path traversal characters then the file path will also result in a bypass to the supposedly contained location.