Potential DoS using lodash/underscore collection methods over user inputs

JavaScript is a magical language. Functions are objects, and objects can have new properties added on the fly. Compilers optimize the code to run it very fast. Even more magic is available with a single npm install command. Let’s take underscore or lodash: they bring a rich set of helpers to provide consistent functional interface for many, many tasks. Map, reduce, each, groupBy, keyBy — these are some of my favorites. Even if you aren’t using these libraries directly, you should look at their source to see what they do and why.

The magic can sometimes bite. I was investigating a rare bug that affected one of Airtable’s users. It manifested itself as an exception

TypeError: Cannot read property 'toLowerCase' of undefined

Our partners at Zapier helped to identify the cause of the error. Using _.each over results returned by the Airtable API leads to iteration over numeric keys when the returned object has a length property with a numeric value. The Airtable API is unique in that the user controls the schema and we provide a RESTful API for that specific base. One user happened to have a numeric length column that triggered this error. The fix was simply change

_.each(object, function(value, key) { ... });

to

_.each(_.keys(object), function(key) { 
   const value = object[key]; 
   ... 
});

Vulnerability

_.each will iterate over numeric keys for array-like objects, i.e. objects that have a length property with numeric value

For lodash, it’s here and here. For underscore, it’s here and here.

The behaviour was highly unexpected and got me thinking. Could it lead to an RCE? Maybe. Could it lead to unintended data disclosure? Maybe, with some esoteric code using it.

Could it lead to a DOS? Oh, that’s easy. Iterating over a very long array will lock up the CPU.

Can we cause a crash? That’s not too hard either. Consider a code that validates user input like this:

const badKeys = [];
_.each(obj, (value, key) => /^key/.test(k) || badKeys.push(k));

Now passing an object with a length property having a large number will lead to OOM (out of memory):

const obj = {"keyFoo": "bar", "length": 1000000000};

Here’s a full example of a vulnerable app using the same pattern: https://github.com/syrnick/underscore_lodash_each_dos_example

If you find another situation or a specific library that’s vulnerable, fill out this form — I’d love to hear about it.

What should you do?

Here’s my personal recommendation:

Step 1. Inspect your code for the use of collection methods over user-controlled data. Replace all of them with iteration over object keys:

_.each(_.keys(object), function(key) { 
    const value = object[key]; 
    ... 
});

If you find yourself vulnerable, fix it asap, minimize the number of changes, and keep code as clear as possible.

A lots of helper methods are vulnerable, but most likely you’re using only a few. In our case, we were using a custom build of lodash, so we knew exactly what methods we were using and inspected them all.

In our case, the vast majority of our call sites weren’t vulnerable to DOS because the objects were keyed by internal IDs. Phew.

Step 2. Follow the discussion on your favorite library. Here’s the thread for underscore. Changing this behavior might require significant changes including a semver major jump.

Step 3. Stop using vulnerable methods altogether. Now that you’ve stopped using vulnerable methods over user data, it’s best to phase them out completely. Figure out proper replacements and roll them out to your whole codebase. E.g. you could create a wrapper that checks for the length attributes and uses _.keys internally. Then you can slowly migrate to it. This way you won’t accidentally re-introduce the vulnerability again.