1

TL;DR: Is it dangerous to have user input used in a require function in Node.js? Can it be used to read files, and if so how can I protect against that?


For example, I have an ExpressJS server running, with an API. The client can make a call to /api/login, which is then required as ./api/login(.coffee):

require('coffee-script')
app = require('express')()

app.all '/api/:func?', (req, res) ->
  # Get API function
  func = req.params.func

  # Check if file exists
  fs = require 'fs'
  fs.access "./api/#{func}.coffee", fs.F_OK & fs.R_OK, (err) ->
    if err
      # Send error when invalid call
      api = require('./api/error')
    else
      # !!! Injection ?
      api = require('./api/' + func)

    # Call API
    api(req, res)

I assume that one can't send a request to http://example.net/api/../../../etc/passwd for example, but I only tested that in my browser, and I'm not sure if my browser handles that. Besides that, I think the worst thing that could happen would be that Node crashes because it can't require such a file.

Are there any security issues here, and how would I block such attacks?

  • blocking access to ../../etc/passwd is a responsibility of the server and not of the browser. – mcgyver5 Jun 8 '16 at 19:27
  • @mcgyver5 I meant that I didn't know if the parenting ../../../.. is done client-wise or server-wise. If it's client-wise, I need to watch out; if ExpressJS already handles this, it should just point to the root of my public directory – Charlie Jun 8 '16 at 19:36
2

If you let users require unexpected files, you will get unexpected behaviour. While it may not be obvious how an attacker could exploit that for anything useful, it is impossible to predict the consequences. That is bad.

At the very least you open up a DoS vulnerability since a user can easily crash Node and force a restart. A few requests like that per second and you are out of business. At worst... who knows?

So what is the solution? I see two possible approaches:

  1. Whitelist. Have an array with allowed file names, and make sure func is in that array before you require.
  2. Make sure you stay in the API folder, and make sure all files there can be safely included. For how to do that, see this question of mine.
| improve this answer | |
  • Thanks! I was planning on using it more like a framework for other things I'm wanting to build, so hard-coding an array would be difficult later on. I'm currently writing a function to store all files in the API folder to an array once, and use that later to check. – Charlie Jun 8 '16 at 19:49
2

You should never use dynamic requires, unless you have heavily sanitised the input (whitelist if possible).

Node.js will accept ANY file extension in its require call. It does not validate it to be a .js file, E.g

require('malicious-uploaded-file')

This 'malicious-uploaded-file' could be a .png|.jpg file filled with malicious JS.

I have previously exploited this flaw using a poorly written file upload system (did not rename files, or have correct dir permissions) that let me upload a .png file filled the Node reverse shell code. I then manipulated a dynamic require to use directory traversal to the uploaded script.

Now this particular system had many flaws but these 2 allowed me to take complete control over of the remote system.

| improve this answer | |
0

Exposing the require function like this is almost as or just as bad as exposing eval

  • require('./api/good.js');eval('//xkcd.com/327');// <- Code Injection
  • require('../../../dev/zero') <- DDoS
  • require('../../../var/upload/bad.js') <- Remote Exploit
  • require('../../../opt/proprietary-package/proprietary-code.js') <- Bad
| improve this answer | |

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.