Lodash: Understanding the recent vulnerability and how we can rally behind packages
Tags: opensource, github, npm, security
Published: 2020-08-04 15:56:17

We all hate it when we get an email from one of our CI/CLI tools complaining about a new vulnerability in one of our dependencies, and if there's no easy patch available, some of us rush to GitHub and write a passive-aggressive GitHub Issue asking a maintainer of the package to prepare a fix, to provide timelines, and to keep communications lines open on the issue. Maintainers already have a lot on their plates, especially if their package becomes mainstream and is utilised by sometimes millions of developers worldwide, like Lodash.


Lodash is a JavaScript library that provides functions for common programming tasks. It is the #1 most used package on NPM, and is being downloaded 34m+ times a week! We use it at huntr.dev, so when we saw that it had a vulnerability, we kept a keen eye and followed the process that ensued.


On April 30th, a Prototype Pollution vulnerability was identified in the most depended upon package on NPM; Lodash. If this is the first time finding this out, don't worry, a fix has been developed and merged into the repo - so you are safe (for now). 


In this post, we will dive into the detail: the disclosure, the fix and then discuss where we as a community could have done better!

<h2>Lodash</h2>

On the 30th April, an issue was opened on the Lodash repository by @nickrobson stating that he had found a prototype pollution vulnerability in lodash.js. The next day he had opened a pull request with a fix for the vulnerability. However, the maintainer only accepted the pull request almost 2 months later.


So why did it take so long for the maintainer to feel comfortable merging the pull request? Before we go into the potential reasons, let's take a look at the vulnerable code and see how it could have affected the users of Lodash (shoutout to our #1 huntr @MufeedVH for the writeup).

<h2>The Prototype Pollution</h2>

Lodash versions prior to 4.17.19 are vulnerable to a Prototype Pollution (CVE-2020-8203). The function <kbd>zipObjectDeep()</kbd> allows a malicious user to modify the prototype of an Object if the property identifiers are user-supplied. To be affected by this issue, developers would have to be zipping objects based upon user-provided property arrays.


This vulnerability causes the addition or modification of an existing property that will exist on all objects and may lead to Denial of Service or Code Execution under certain circumstances.

<h3>Deep dive Into The Vulnerability</h3>

The vulnerability arises when we give a maliciously crafted object to the <kbd>zipObjectDeep()</kbd> function in Lodash.

<h4>The Vulnerable Code:</h4>

```
function zipObjectDeep(props, values) {
  return baseZipObject(props || [], values || [], baseSet);
}
```

When we call the function <kbd>zipObjectDeep()</kbd>, it passes the input object and an assigned function to <kbd>baseZipObject()</kbd> function, which contains the code:

```
function baseZipObject(props, values, assignFunc) {
  var index = -1,
      length = props.length,
      valsLength = values.length,
      result = {};

  while (++index < length) {
    var value = index < valsLength ? values[index] : undefined;
    assignFunc(result, props[index], value);
  }
  return result;
```

The assigned function given to <kbd>baseZipObject()</kbd> function is <kbd>baseSet</kbd> which is the function modifying the prototype. This function doesn't have any validation of the input object, making it possible to inject a prototype and causing a Prototype Pollution Attack.


This is how the vulnerable function <kbd>baseSet()</kbd> was implemented:

```
function baseSet(object, path, value, customizer) {
  if (!isObject(object)) {
    return object;
  }
  path = castPath(path, object);

  var index = -1,
      length = path.length,
      lastIndex = length - 1,
      nested = object;

  while (nested != null && ++index < length) {
    var key = toKey(path[index]),
        newValue = value;

    if (index != lastIndex) {
      var objValue = nested[key];
      newValue = customizer ? customizer(objValue, key, nested) : undefined;
      if (newValue === undefined) {
        newValue = isObject(objValue)
          ? objValue
          : (isIndex(path[index + 1]) ? [] : {});
      }
    }
    assignValue(nested, key, newValue);
    nested = nested[key];
  }
  return object;
}
```

The input object is not properly validated and is then used to assign values and keys. If we give a maliciously crafted object with a prototype, we can pollute the object and it can lead to Denial of Service or Code Execution.

<h3>Exploiting the vulnerability</h3>

As the function didn't have any validation, just passing a basic prototype is enough to demonstrate/exploit the vulnerability.

<h4>Proof of Concept:</h4>

Let's give a maliciously crafted prototype as the input to the function <kbd>zipObjectDeep()</kbd>.

```
const _ = require('lodash'); // npm i lodash@4.17.15

var huntr = {
    nickname: 'Mufeed VH',
    username: 'mufeedvh'
}

_.zipObjectDeep(['{}.__proto__.isAdmin'],[true])

console.log(huntr) 
// { nickname: 'Mufeed VH', username: 'mufeedvh' }

console.log(huntr.isAdmin)
// true
```

﻿The above-crafted code shows how with a user-provided input, we can manipulate the prototype of Object and inject a fraudulent <kbd>isAdmin</kbd> flag. This can be dangerous in many ways, bypassing AuthN/AuthO implementations, executing arbitrary code and other creative ways that a hacker could come up with.

Now before reading on, we shouldn't look to point the finger, especially not the maintainers of Lodash. If you look back at the commit logs, they have been updating, fixing bugs, and maintaining the package since the beginning.

<h4>Aware but don't care</h4>

So here's the controversial statement (if you want to weigh in and give your opinion on this please do in the comments). On the 1st May @jdalton moved the issue to a separate thread and responded with:

<a href="https://ibb.co/kJMtQ32"><img src="https://i.ibb.co/Lrv3nQd/jdalton.png" alt="jdalton" border="0"></a>

He then shut the thread to the public allowing only maintainers to comment. Whether he could have said it better is neither here nor there, but shortly after this, many of Lodashs' users were posting on the original issue asking for one of the fixes to be merged:

<a href="https://ibb.co/QbFVn6c"><img src="https://i.ibb.co/f1YWtCS/gjoseph.png" alt="gjoseph" border="0"></a>

<a href="https://ibb.co/NY3CM3h"><img src="https://i.ibb.co/kcyBtyn/syhan.png" alt="syhan" border="0"></a>

<a href="https://ibb.co/QbFVn6c"><img src="https://i.ibb.co/f1YWtCS/gjoseph.png" alt="gjoseph" border="0"></a>

It's also important to note here that it's unclear that of the 117,952 projects (as @martin-walsh highlighted), how many of these do depend-upon user provided property arrays when interacting with zipObjectDeep and what can these projects do, given the locking of the thread and the delaying of the PR acceptance - what if @jdalton had serious concerns with the proposed PR?

<h4>Why was it not merged?</h4>

Before going into the technicality of the fix itself, let's assume it was sufficient, so why was it not merged? Here are a few reasons we've seen in similar situations:


<ol>
    <li>The maintainer did not have the time to check the fix
        <ul>
            <li>The maintainer is on holiday</li>
            <li>Has just had a family grievance</li>
            <li>Is simply taking a break</li>
        </ul>
    <li>Maintainers aren't security-minded
        <ul>
            <li>Maintainers who aren't well versed in security are likely not well equipped to check and remediate these issues</li>
        
        </ul>
    </li>
    <li>No incentive for the maintainer to act
        <ul>
            <li>The maintainer has a tech roadmap that he is looking to follow, he is deep into his sprint and is focused on developing new features, and so, this isn't a priority</li>
          
        </ul>
    </li>
<li>'Too many cooks spoil the broth'
        <ul>
            <li>The fix was written by the original discloser, but shortly after, Snyk got involved and developed a different fix, then a report was published on Hackerone, and even SourceClear and of-course the NPM advisory team were involved.</li>
          
        </ul>
    </li>
</ol>

<h4>Why we shouldn't complain</h4>

Your first reaction may be 'wow! how could a maintainer ignore this?!', 'how could he say that?!' but in truth, we should not be complaining... Lodash is free — it's maintained by contributors for free and utilised by millions of developers for free — why should the maintainer act, what's in it for them?


Your all devs here, so what is currently on your desk? In your pipeline? What's the problem you have spent the last 3 hours trying to fix? and then imagine someone comes over, taps you on the shoulder and says, "stop what you're doing, you have another problem that's more important." Would you stop what you were doing and do what they ask of you?


We need to take some of that burden off of maintainers shoulders, helping them as much as we can to get security issues fixed as quickly as possible - this is where huntr.dev comes in (which we will talk about briefly later).

<h3>The Fix</h3>

To fix Prototype Pollution Attacks, there are multiple ways. We can fix it by freezing the Object with the JavaScript ES5 function <kbd>Object.freeze()</kbd> or by defining a null Object <kbd>Object.create(null)</kbd>. The other way to fix this vulnerability is to validate the input to check for added prototypes. The implementation can vary according to different use cases.


(To find out more about Prototype Pollution Attacks and it's mitigations, check out this paper: [Prototype pollution attack in NodeJS application](https://github.com/HoLyVieR/prototype-pollution-nsec18/blob/master/paper/JavaScript_prototype_pollution_attack_in_NodeJS.pdf))


Lodash fixed this issue by validating the input Object to check for prototypes. Here is the fixed code:

```
function baseSet(object, path, value, customizer) {
  if (!isObject(object)) {
    return object;
  }
  path = castPath(path, object);

  var index = -1,
      length = path.length,
      lastIndex = length - 1,
      nested = object;

  while (nested != null && ++index < length) {
    var key = toKey(path[index]),
        newValue = value;


+   if (key === '__proto__' || key === 'constructor' || key === 'prototype') {
+     return object;
+   }

    if (index != lastIndex) {
      var objValue = nested[key];
      newValue = customizer ? customizer(objValue, key, nested) : undefined;
      if (newValue === undefined) {
        newValue = isObject(objValue)
          ? objValue
          : (isIndex(path[index + 1]) ? [] : {});
      }
    }
    assignValue(nested, key, newValue);
    nested = nested[key];
  }
  return object;
}
```
Reference: [Fix Commit](https://github.com/lodash/lodash/commit/c84fe82760fb2d3e03a63379b297a1cc1a2fce12)


We can see that the patch on the function <kbd>baseSet()</kbd>, has implemented a simple validation to check for prototype keywords (<kbd>__proto__</kbd>, <kbd>constructor</kbd> and <kbd>prototype</kbd>), where if it exists, the function returns the object without modifying it, thus fixing the Prototype Pollution Vulnerability.

<h3>An alternative</h3>

So if the aim is to reduce the time from point of disclosure to point of the merge, is there a way we as a community can help?

[<h3>huntr.dev</h3>](https://huntr.dev)

Crowdsourced security for open source. We have seen the open source vulnerability landscape increase drastically over the past few years, and it looks like in a lot of areas it is only going up. We aim to help maintainers mitigate some of these issues, giving both them and there users peace of mind knowing that there is a body that is not only incentivising security disclosures, but also the remediation of the vulnerability and even the acceptance of the pull request.


By leveraging community sheriffs with backgrounds in security, we are constantly reviewing disclosures across the whole open source eco-system, collating all of the information, developing a sufficient fix, and providing it to the package owners in an easy to understand way.


When we find something in a package you are a maintainer of, we will notify you, and open up a bounty to get a fix created as soon as possible! Usually, these programs ask you to sponsor, give up some money, but what we ask is for you to communicate, when you see us pop up on your repository, if you disagree with our findings, let us know, let's discuss it in the open, so everyone can chip in. And if you are happy with a fix, all we ask is you don't leave your users waiting and accept the pull request as early as possible!


The ultimate aim is to reduce the average time it takes to fix vulnerabilities across the whole open source ecosystem. It's a mammoth task, but one we are willing to take on, and will not give up until it is beaten.

<h4>Contact</h4>

Want to be part of the journey? Sign up at huntr.dev, find and fix vulnerabilities in open source, and build your security profile.


And if your a maintainer who could use some help with outstanding vulnerabilities, then send an email to info@huntr.dev and we will rally the community to try and create a fix!


References
https://github.com/lodash/lodash/issues/4744
https://github.com/lodash/lodash/commit/c84fe82760fb2d3e03a63379b297a1cc1a2fce12
https://www.npmjs.com/advisories/1523
https://nvd.nist.gov/vuln/detail/CVE-2020-8203
https://hackerone.com/reports/712065