Imagine a typical day at work where you help a colleague review a Node.js codebase. You hopefully apply some secure code review skills to identify potential security vulnerabilities. But can you find all the vulnerabilities in a given pull request? What about the security issues that might arise later in the future from the same code change if a refactor takes place?

Let’s put this secure code review to the test. Following is a Node.js code snippet from your favorite framework, whether Express, Fastify, or otherwise.

Take a look at this server-side JavaScript code and before you dive into the next section of a collaborative secure code review, try your best to spot any potential security vulnerabilities:

// save user's avatar to images/ directory
const IMAGE_DIR = '/src/tmp/images'
app.post('/upload', async (req, res) => {
  const uploadedFile = req.files.avatar
  const fileName = uploadedFile.name
  const fileContent = await uploadedFile.data()
  const contentHash = Util.hashContent(uploadedFile.name)
  const filePath = Path.createAbsolutePath(`${IMAGE_DIR}/${contentHash}`)
  if (!await filePath.exists()) {
    await FS.writeFile(filePath, fileContent);
    return res.send('File uploaded!')
  }
  return res.status(400).send()
})

Node.js Secure Code Review

I shared a similar Node.js pseudo code snippet on social (X, Bluesky, and LinkedIn) and asked friends to provide feedback on what they thought about the code.

Let’s dive head on and break apart the above Node.js code snippet for the code review comments I received:

Large File Uploads in Node.js

One friend commented wouldn't the thread hang for very large files?. Valid point indeed. In the code snippet specifically, the Promise syntax won’t hang JavaScript’s main event loop thread, but processing large files through a Promise can still cause issues around memory handling. A better approach would be to utilize Node.js streams API and stream the file content to disk directly.

Upload Storage Capacity

In continuation to the above, one could also raise the question about free storage capacity on the server and whether the server has enough disk space to begin with, to store the uploaded files, particularly in the designated /src/tmp/images directory.

Rate Limits in Node.js

Both of the above comments around large file handling and upload storage capacity lead to a higher-level concept of rate limiting.

What is Rate Limiting? Rate limiting is a security measure to prevent abuse of a service (such as an exposed Node.js API) by limiting the number of requests a client or API consumer can make in a given time frame.

In Node.js, you can implement rate limiting within the Node.js web application server or in the reverse proxy server (such as Nginx or Apache) that often in production workloads sits in front of the Node.js server. If you’re looking for a quick rate limiting solution to an Express-powered Node.js server you can use the express-rate-limit npm package.

Time of Check to Time of Use (TOCTOU) Vulnerability

This is a bit of an advanced topic but worth digging into.

Not surprising to receive this feedback from a security-minded friend. Vladimir from Microsoft’s research team commented that perhaps time of check / time of use issue may arise between the time that passed while checking if a file exists and writing it. This could possibly leave a gap for race conditions if this can be executed in parallel.

File Upload Validation in Node.js

Another friend, Tomer, head of security research at F5 also pointed out the mandatory need for file upload validation. In the code snippet, the uploaded file is not validated for its file type, size, or content. This could lead to a potential security vulnerability such as a file upload vulnerability.

For example, a file type can be determined by either a file extension check (which is more often than not mistaken to be based on a string matching of the extension string) or by checking the file’s magic bytes. Magic bytes are the first few bytes of a file that can be used to determine the file type. For example, a PNG file’s magic bytes are 89 50 4E 47 0D 0A 1A 0A.

File Upload Privacy and File Upload Sanitization

An interesting aspect of handling user uploads could be around keeping user’s privacy a top concern and mixing that with sanitization of metadata in the uploaded files from being a potential source-to-sink attack vector.

Let’s say you’re building a social media platform and users can upload their profile pictures. You’d want to make sure that the uploaded image doesn’t contain any metadata that could potentially leak sensitive information about the user. Pictures taken with a camera or a phone could contain metadata such as GPS coordinates, camera model and other tidbits that could be reverse engineered using OSINT techniques to identify information about the user who took the picture.

Hash Collision in File Uploads

Let’s look at the relevant part of the code snippet:

const fileContent = await uploadedFile.data()
const contentHash = Util.hashContent(uploadedFile.name)

The file name being written to disk is based on a hash of the file name. What happens if another user uploads a file with the same name? What happens if two different file names end up having the same hash?

These could potentially lead to a hash collision. Is it a better approach to hash the file content itself rather than the file name to ensure uniqueness? Imagine the following:

const fileContent = await uploadedFile.data()
const contentHash = Util.hashContent(fileContent)

What’s the issue now?

File Permissions

Ok so you’re writing files to disk in the /src/tmp/images directory. What are the file permissions set on the directory? Do those belong to the Node.js server user? Is that a good idea?

Soumen who runs Security Governance at Barco raised this concern too, which many developers don’t often think about because they’re not aware of file systems.

File Upload Size Edge Cases

What happens if there’s an edge case where the file contents is empty and effectively the file size is 0. First off, remember we raised a prior concern during code review about a using a unique file name based on hashing the file’s content? Well, that’s not going to work well here.

Think about edge cases, people!

Cross-site Scripting (XSS) in File Uploads

Ugh, yes. The one that’s so commonly overlooked. What does a client-side vulnerability such as Cross-site Scripting (XSS) have to do with server-side file uploads? Isn’t that the client’s problem? Well, kind of but also, “depends”.

Let’s say the attacker manipulated the file name so that the file name has special HTML characters in it. What if the file name is set by the attacker to be hello-<script>alert('XSS')</script>.pdf?

Now, imagine that the client-side takes the file metadata, such as the file name, as part of the Node.js API server response and displays it on the client-side. The frontend developer might commonly mistake to think that if this is coming from the server, especially something as seemingly harmless as a file name, it would be safe to render on the client-side.

If the file name is not properly escaped in the client-side, this effortlessly turns into an attack vector that manifests as an XSS vulnerability.

File Upload Persistence

How about we take a step back and think about the persistence of the uploaded files?

In the provided code snippet, the uploaded files are placed in the /src/tmp/images directory. What happens if the server crashes? What if you need to spin it up or down? are you managing mounted file paths properly?

A better cloud-native development approach would be to store the uploaded files directly in the cloud via services like Amazon S3, Google Cloud Storage, or Azure Blob Storage. This way, you can ensure that the uploaded files are persisted and available even if the server crashes. This also has other benefits like scalability, durability, performance, and even security!

So yes, consider uploading the file directly to cloud storage.

The Security Bug You Totally Missed: Path Traversal in Node.js

Not one person at all, through-out all of the different platforms I shared this Node.js code snippet on, mentioned the potential security vulnerability that exists in the code snippet - Path Traversal.

“Wait, what? There’s no path traversal vulnerability here!” you might say.

Let’s see if you’re right. Look again at the code snippet:

  const fileName = uploadedFile.name
  const fileContent = await uploadedFile.data()
  const contentHash = Util.hashContent(uploadedFile.name)
  const filePath = Path.createAbsolutePath(`${IMAGE_DIR}/${contentHash}`)

If the file name is ../ or %2e%2e%2f (URL encoded ../), none of these would result in a path traversal vulnerability because the file name is hashed into an alpha numeric string (depends on the hash function, but you get the point). So if the file name input is ../../../../../etc/passwd then the effective contentHash would be ab15sb176bs61bs7b9sbs71gs71b and the file path would be /src/tmp/images/ab15sb176bs61bs7b9sbs71gs71b.

So where is the file path traversal vulnerability, you ask?

Right now there is no path traversal. The root problem with path traversal happens due to how paths are constructed and used in the application. RIGHT NOW the file path is constructed by concatenating the IMAGE_DIR and the contentHash. This is the last thing you want to do. You never concatenate paths together to form a file path. That’s the root cause of path traversal vulnerabilities.

What happens if in a future code refactor, the product manager decides it is more user-friendly to store the uploaded files in a directory structure that mirrors the user’s email address? or keeps the original file name to allow for a nice friendly download experience to users? Now, the file name will be derived from a user-controlled input and leaving the rest of the code as is, which means concatenating a directory with a file name is a recipe for disaster.

Learn about Secure Coding and Path Traversal Vulnerabilities

If you are serious about polishing your secure coding skills, go grab a copy of my Node.js Secure Coding book that teaches you how to defend against path traversal vulnerabilities from the inside out and how to build secure Node.js applications.