Monday, February 13, 2012

NodeJS: Switch is EVIL

switch statement in JavaScript is known to have bad effects as in other programming languages. In this post we discuss it's potential impact in server side JavaScript context like NodeJS. For more history on switch please refer Douglas Crockford's YUI blog post.

Let's look at a sample code snippet as in the screenshot below. This is an over-simplistic example. It is a funny little take on an app that reveals it's users the discount code based on their tiers. The logic that will determine the tier of the user and it's category is omitted for benefit of stressing on the issue at hand.

What should have happened was, the basic tier user Valued Customer should have been shown only 10% discount code. Now since our programmer forgot to apply the brakes (i.e. break highlighted in red in the previous case - in hurry or just human error or insufficient knowledge of switch may be), the second case code under case (dis < 5000) triggered leading to giving higher discount to a basic tier customer and showing a not so good message, as in the screenshot below.

Still in this fun app nothing really nasty happened. And the idea was exactly that to take a simple code and demo what switch could lead to.

In real world a similar mistake could lead to serious vulnerabilities - those are hard to detect. More I think of JavaScript, more I believe, coding best practices usually translate to security best practices. To be safe, anti-patterns like implied globals, with, eval, should be avoided. 

NodeJS: 'with' is evil

It is a known fact that with statement in JavaScript is evil. For a good read on why read Douglas Crockford's post on YUI blog.

Let's look at how it implies on server side JavaScript. Below is a fun little app coded by a beginner that tries to be funny although in real apps this could lead to unbelievably serious vulnerabilities.

So what went wrong here? The developer loves using with for it's shot handedness and thought she called the property names of the welcome object correctly. Also it didn't show any errors. But what her first user on the web saw was this (not that this)

So, she did a typo and ended up unintentionally modifying global variables she wasn't even aware of. Let's just imagine they existed in some other code base where she couldn't even see. This just reminds me how difficult will it be for a security guy like me to code review a code with with.

Now how with works is, it tries to find the property assignments in the context of the called object, if found, great, else it tracks back on the higher scope till reaching the global scope and assigning (actually clobbering) value of some other global variable if there is a match. Think common names like i, x, a, name... we all grew up coding with (not that with).

In short, do not use with, unless you are very sure of what you are doing. On a positive note, use of with is forbidden in ES5 strict mode.

NodeJS: Global Namespace Pollution

From a security standpoint, a big change for most server-side developers moving on to NodeJS would be the notion of JavaScript's global namespace. If misunderstood or with limited knowledge of this inherent property, writing secure NodeJS web apps will be a challenge.

So what is it? A prime property of JavaScript is, it is a 'global' language.
  • variables by default have an implied global scope
  • functions by default have an implied global scope
  • all objects inherit from the native / built-in global objects
Let's understand more with a code snippet. In a traditional PHP script (or any other non-server side JS paradigm), each request  has it's own scope. So a code similar to below will always print 1, unlike in the case of NodeJS. Any request will share the same global scope.

In relevance to this code, each request will increase the global variable gbl by 1, as seen in the screenshot below for two different requests. In a PHP script such a model would only show 1 for every request.

So, what could go wrong from security perspective? Short answer - it depends, on the context and sensitivity of a global variable or function. An attacker could exploit this behavior to her benefit to achieve desired effects. What could those be,
  • as a web user, could bypass logic flows
  • a malicious library could over-ride native, built-in or known objects, variables, functions to adversely impact sensitive code base/libraries
  • in a shared coding environment, an inexperienced developer could unintentionally over-ride native, built-in or known objects, variables, functions - adversely impacting sensitive code base/libraries
A lot more serious stuff could happen only time will tell.

So what's the defense? Unless really needed, always define your functions, variables, as local, as shown in the screenshot below.

Now you get the desired effect as in PHP. Each request now shows gbl as 1. For potential rogue/malicious libraries - audit them! JSLint (though a bit noisy) is a good bet.

I am a JavaScript beginner, hence for a healthy advise for typical programming requirements, I recommend reading Douglas Crockford's post on why Global is Evil and the best practices to avoid it.