When This is Really That

A Brief Review

For many, this section will be a review, so feel free to skip ahead. However, since we are dealing with some features of the JavaScript language that most of us don’t use every day, I’m going to go ahead and give a brief refresher so the rest of this makes sense.

Object oriented programming in JavaScript can be a little bit unconventional. Typically, private methods and variables are expressed as locally declared variables, while public methods and variables are expressed as properties attached to the this object, which is an object that refers to current variable scope.

This pattern fits well enough for most use cases, but what about the situation where you need a piece of data that is publicly visible, but only privately writable, such as in the case of the length property of builtins?

Typically what happens is that the property is defined with the __defineSetter__ and __defineGetter__ methods, which allow a function to basically act as a gatekeeper to the property - denying write access in certain cases, and allowing it in others.

But what if this could be modified? Enter Function.bind – inherited from the function prototype, it is available on every function in JavaScript by default. It’s purpose is to take an existing function and call the function with a this object specified when bind is called.

Can touch this

In many cases, native functions which access memory directly via v8 bindings have their memory access protected by length limits and metadata that exist inside this, as mentioned above. The length property of a Buffer is one example.

Well, we know now that function.bind can tamper with those pieces of metadata, which are used to handle special types of memory access when it comes to builtin modules and types.

In some modules this is handled sensibly – internal functions check to make sure that this is a valid instance of the object it expects, and overrides any attempts to modify those special internal properties without changing the memory the object has allocated. As far as I know, all builtins in v8 use this method to protect sensitive data.

In Node.js, however, the majority of the added features have no protection against this kind of attack.

You can allocate a buffer with a length of 100, and fill it with 10000 characters if you want, by binding the Buffer prototype to a tampered context as shown in this example:

1
2
3
4
5
6
7
var t = new Buffer(100)
ctx = t.__proto__
ctx.length = 100000
t.tamperedToString = t.toString.bind(ctx)
t.tamperedToString()
Assertion failed: (obj_data != NULL), function StringSlice, file ../src/node_buffer.cc, line 262.
Abort trap: 6

Here we tried to access unallocated memory, failed with a triggered assertion, and then crashed the process.

You Can’t Trust What You Don’t Control

The next issue is that the Node.js core modules are inconsistent with their protection in regards to verifying the integrity of the util module. Some core modules create a local instantiation of critical functions used to check types, because they can have undesirable impacts if they are tampered with and the module is using the globally cached version.

There are also modules that do not and simply use the global version. Once again, thebuffer module lies in the second group.

Buffer uses util.isArray internally to check and see if the data passed into the constructor is an array. If it is, it then pulls the length property from the array and trusts it as the actual length of the data that it is going to be storing in the buffer.

But if util.isArray has been overwritten, it is possible to pass in an object that meets the internal requirements for a valid “array,” yet is free from the safety constraints placed on a real array – namely that the length property matches the length of the data.

Example:

1
2
utils.isArray = function(){return true;}
var x = new Buffer({length:0x3FFFFFFF, data: new Buffer(1000)})

If you open up your console and play around with the above example you might notice that you can’t really write anything to the buffer. Once again, limited usefulness, but certainly buggy behavior.

Bringing it All Together

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
var util = require('util');
util.isArray = function(){return true}
ctx = Object.keys(Buffer.prototype)
bctx={};ctx.forEach(function(key){bctx[key]=Buffer.prototype[key]})
bctx.length=1000000
bctx.__defineGetter__('length',function(){return 10000})
bctx.__defineSetter__('length',function(){return 10000})
dat = new Array(10000).forEach(function(x,i){
    var tmp = new Array(256);
    tmp.forEach(function(e,j){
    tmp[j]=process.argv[2];
    });
    dat[i]=tmp;
})
y=Buffer.bind(bctx)
c=y({length:0x3FFFFFFF, data: dat})
console.log(process.pid);
console.log(c.toString())

Walking through what’s going on above we see:

  • Overwriting isArray to allow passing in an object with an incorrect length
  • Make copies of all the prototypal methods on Buffer - a direct copy won’t do, it’ll just end up messing with the global Buffer object and prevent you from proceeding
  • Add a length property, and define getters/setters on the length to ensure that Buffer can’t update the length when it needs to
  • Fill an array at some fraction of the expected size with garbage
  • Bind the Buffer constructor to the tampered context
  • Create a new Buffer instance with the binding, passing in the object masquerading as an array
  • Enumerate the Buffer with .toString(), which relies heavily on this.length

At this point, the process will stop responding while filling up memory and terminate with SIGILL. Crashwrangler, although not 100% accurate, reports this to be an exploitable bug.

I contacted the Node.js Security team and reported the issue – it will not be fixed. Given how specific the situation has to be in order to exploit, the only likely situation in which this could be exploited is in a sandbox implementation, which I mentioned when reporting. As a sandbox in Node.js is unlikely to ever be safe, this was deemed to be too low of a priority to be worth the dev teams time.

Update: I have had additional discussion with the Node.js team, and they are giving the issue additional consideration. I am still uncertain as to whether or not a fix will be issued, but from our conversation, it appears that it will be fixed at some point. They have been one of the most responsible OSS Security teams I’ve had the pleasure of working with, and this post is in no way intended to be critical of them.

Why it Matters

The impact here is that a useful Node.js sandbox without VM/OS level isolation will never be secure.

Even if the context is locked down to remove privileged features, the builtins added with Node.js add a significant risk for a sandbox breakout exploit. Combined with the response from the dev team that security measures primarily affecting sandboxes will not be fixed, the best thing to do here is to use another language for this purpose, or rely on isolation implemented outside of the language, such as a VM.