Security: README diskStorage example uses Math.random() for filenames instead of crypto-safe random

[ekreloff]

Summary

The library's internal default filename generator correctly uses crypto.randomBytes(16) for cryptographically secure filenames (in storage/disk.js:6-9):

function getFilename (req, file, cb) {
  crypto.randomBytes(16, function (err, raw) {
    cb(err, err ? undefined : raw.toString('hex'))
  })
}

However, the README's custom diskStorage example teaches a pattern using Math.random():

filename: function (req, file, cb) {
    const uniqueSuffix = Date.now() + '-' + Math.round(Math.random() * 1E9)
    cb(null, file.fieldname + '-' + uniqueSuffix)
}

This is the example developers copy when they need custom filenames (e.g., preserving file extensions, adding field-based prefixes) — which is almost every production use case.

Why this matters

1. Math.random() is not cryptographically secure. V8's xorshift128+ PRNG state can be recovered from a small number of outputs (reference). Math.round(Math.random() * 1E9) provides ~30 bits of entropy per filename.

2. Date.now() narrows the window. If an attacker knows approximately when a file was uploaded (within ~1 second), Date.now() contributes ~0 bits of additional unpredictability, leaving only the Math.random() component.

3. Filename enumeration enables unauthorized access. If uploads are served from a web-accessible directory — a common Express pattern — an attacker can enumerate possible filenames to access other users' uploads without authorization.

The library already knows the right way to do this (its own default uses crypto.randomBytes). The README just teaches the wrong pattern.

Reproduction

// README pattern: predictable
const uniqueSuffix = Date.now() + '-' + Math.round(Math.random() * 1E9)
// Example output: "avatar-1711817423456-482917364"
// Attacker who knows the ~1s upload window only needs to try ~1B values

// Library default: secure
const crypto = require('crypto')
crypto.randomBytes(16, (err, raw) => {
  console.log(raw.toString('hex'))
  // Example: "a3f8c2e91b7d40f5e6a2c8d3b1e7f9a0"
  // 128 bits of entropy — infeasible to enumerate
})

Suggested fix

Update the README example to match the library's own approach:

const crypto = require('crypto')

const storage = multer.diskStorage({
  destination: function (req, file, cb) {
    cb(null, '/tmp/my-uploads')
  },
  filename: function (req, file, cb) {
    crypto.randomBytes(16, function (err, raw) {
      if (err) return cb(err)
      cb(null, file.fieldname + '-' + raw.toString('hex'))
    })
  }
})

This is a one-line change with zero API impact — the filename callback already supports async via the callback pattern.

Impact assessment

Factor	Detail
Severity	Medium — default is secure; only affects custom diskStorage users
Scope	Broad — the README is the canonical reference for custom filenames; this pattern is widely copied in tutorials and Stack Overflow answers
Fix effort	Minimal — README-only change, no code modification needed

Secondary note: examples missing file size limits

The basic examples all use multer({ dest: 'uploads/' }) without setting limits.fileSize. While the README's limits section mentions DoS protection, none of the example code demonstrates it. Adding limits: { fileSize: 5 * 1024 * 1024 } (or similar) to at least one example would help developers remember to set resource limits.

[ekreloff]

This issue is part of a broader pattern I've documented after reviewing three high-profile npm libraries (node-jsonwebtoken, cors, multer — ~115M weekly downloads combined): libraries with secure code defaults that teach insecure patterns in their documentation.

In each case, the library itself handles security correctly, but the README examples demonstrate patterns that introduce vulnerabilities when copied by developers.

Full analysis: The Documentation Attack Surface