from Hacker News

Npm security post-mortem

by IsaacSchlueter on 3/21/14, 6:24 PM with 63 comments

by cyphunk on 3/22/14, 10:41 AM

> * Before they could start, we had a very serious security vulnerability responsibly disclosed by Will Farrington and Charlie Somerville

> * We fixed it on February 17th

the fix scares the shit out of me:

https://github.com/isaacs/st/commit/5a0c1886737a20d78ae00b61...

     Properly escape all relevant html entities
     Avoid problems with files named things like '<img>' and so on.
     
     -      var name = f.replace(/"/g, '&quot;')
     +      var name = f
     +          .replace(/"/g, '&quot;')
     +          .replace(/>/g, '&lt;')
     +          .replace(/</g, '&gt;')
     +          .replace(/'/g, '&#39;')

by wycats on 3/21/14, 7:40 PM
It's worth noting that the XSS vulnerability ("A user could inject scripts into the npm website via the README and license fields") assuredly exposed a whole slew of easy-to-exploit vulnerabilities, and the community should feel very lucky that such an obvious vulnerability was in the wild for so long without being exploited.
TL;DR always use a templating engine that makes you think about XSS and don't allow unsanitized user-provided HTML through raw.
by ilaksh on 3/21/14, 8:34 PM
Why do they have to apologize for that? Almost every piece of software has security vulnerabilities.
Do people now really believe that there are definitely no security vulnerabilities related to the npm registry? Or any other type of registry, or website or application for that matter?
People have completely unrealistic expectations about security. Every time you had a significant amount of new functionality, or even a very insignificant amount, you could have introduced a new security vulnerability.
Basically this other company Lift or whatever could have two full time engineers doing security audits for npm from here until npm is done, and you could still have some other hacker who was thinking differently come up with a vulnerability in some new feature that they missed.
Its great to have the attitude that you are going to make a serious effort, but totally unrealistic to think that you are going to do one security audit and then there will be no more vulnerabilities. And also really makes no sense to make a big deal about it or give them grief or for them to even feel bad. If you think that way then you don't understand security.
You see all of these large projects having security issues not because they are full of negligent or sloppy engineers, but because security takes a lot of resources and is very difficult. The security firms will certainly suggest that engineers are negligent, of course. That ensures that they will continue to get new clients. But the reality is even with a lot of resources dedicated to just security, projects can easily have new vulnerabilities.
So that's great that they are getting regular security audits now. They are ahead of the curve.
EDIT: I notice I have been buried without anyone bothering to even respond. If you disagree, say why you disagree.
by jeremymcanally on 3/21/14, 7:33 PM
Nice write-up and good on them for fixing that quickly, but it's a serious bummer they unnecessarily bring in the RubyGems incident as some sort of awkward "Well at least we didn't screw up that badly!" swipe. It's not relevant to anything else they said.
by akadien on 3/21/14, 7:19 PM
The world would be a little better if every software company could inform its users of security vulnerabilities and bugs as these guys did.
by abecedarius on 3/22/14, 1:21 AM
The 'we fixed it' link points to four new lines including these:
```
      .replace(/>/g, '&lt;')
      .replace(/</g, '&gt;')
```
I can't really tell without more background and context, but I'm surprised this doesn't turn > into > and < into <. Is this a mistake? The same code's still in the HEAD.
by outside1234 on 3/21/14, 7:08 PM
Its great to finally have full time folks managing npm and to finally have the resources to do things right (eg. hire ^lift).
Thanks for doing!
by cjbprime on 3/21/14, 8:03 PM
Nice disclosure, and can't fault the npmjs team given that they commissioned a security audit as soon as they possibly could.
I wonder if the npm, Inc. team told ^Lift about the disclosed vulns before ^Lift's own audit completed. I can imagine being tempted to see whether they'd discover it themselves, to gain more evidence on how comprehensive the audit was.
by maxjus on 3/21/14, 9:52 PM
Y'all might want to limit the number of characters in a user's name...
https://www.npmjs.org/~maxj
by giovannibajo1 on 3/21/14, 7:35 PM
can anybody disclose some figures on how much ^lift (or competitors) costs, e.g.: for a 100K-line Python codebase? A rough ballpark would help a lot.
by jmspring on 3/21/14, 9:42 PM
So the audit was mostly about nom the website and the service for maintaining npm packages? That's a good first step.
Has there been any talk within the node community about auditing node modules themselves? Maybe start with the most popular? I could see this being popular with enterprise development, etc.
I want to say that Strong Loop made noise about doing something like this, but I haven't seen much on it of late.
by dantiberian on 3/22/14, 1:02 AM
Is this why npm made the changes with self signed certificates http://blog.npmjs.org/post/78085451721/npms-self-signed-cert... or is that unrelated?
by filipedeschamps on 3/22/14, 1:08 PM
One of the things I love about Isaac is his empathy. Reading his blog posts, listening to his podcasts on Nodeup, looks like he is writing/talking to me, and since I'm a developer, this makes a huge difference in my motivation.
Great job guys and very responsible choices.
by qubyte on 3/22/14, 2:14 AM
Security postmortems are a great idea. Until time machines happen.
by seaghost on 3/21/14, 9:02 PM
Great read!
by Fasebook on 3/21/14, 9:11 PM
Nearly Paid Muppets?