Incorrect advisory https://nodesecurity.io/advisories/67

writeup.md

https://nodesecurity.io/advisories/67

@xc0nradx

@nodesecurity @feross @mafintosh

Description

The writeup implies that the websocket server will receive the number 50, and by type confusion accidentally allocate an uninitialized buffer of size 50. The buffer will then be sent to the client as the pong response. Using this bug, a malicious actor can potentially read memory off vulnerable servers.

But, after playing around with the PoC, I don’t think this is correct. I think the actual problem is in the client. If you track down client.ping, you’ll see that calling client.ping(50) actually allocates the buffer and sends it to the server. So the server is simply responding with the clients memory buffer. The server never allocates 50 bytes of uninitialized memory and sends it back to the client. It's doing exactly what it should be doing.

To prove this, we're going to print out exactly what the client is sending, and what the server receives.

Demo

Using ws: "1.0.0"

Server PoC:

var ws = require('ws').Server;

var WebSocketServer = require('ws').Server;
var wss = new WebSocketServer({ port: 8085 });

wss.on('connection', function connection(ws) {
  ws.on('message', function incoming(message) {
    console.log('received: %s', message);
  });

  ws.send('something');
});

console.log("websocket server listening on :8085")

Client PoC:

var ws = require('ws')

var client = new ws('ws://demo.server.com:8085')

client.on('open', function () {
  console.log('open')
  client.ping(50) // this makes the server return a non-zeroed buffer of 50 bytes

  client.on('pong', function (data) {
    console.log('got pong');
    console.log(data) // a non-zeroed out allocated buffer returned from the server
  })
})

Then modify line 647 of the server’s ws package to print out the unmasked data. https://github.com/websockets/ws/blob/753937ff1ddc0938513267b4d6d5139a6ad41746/lib/Receiver.js#L647

Then modify line 158 of the client’s ws package to print out the payload before it is sent to the server. https://github.com/websockets/ws/blob/753937ff1ddc0938513267b4d6d5139a6ad41746/lib/Sender.js#L158

Running the PoC you should see something similar to:

This means that the problem was actually in client.ping, not the server’s pong response.

The Fix

Even though the advisory is wrong, the fix is still correct. https://github.com/websockets/ws/commit/29293ed11b679e0366fa0f6bb9310b330dafd795

The fix converts the client.ping(50) into essentially client.ping(‘50’) inside of the sender.

Contact

Stuart Larsen, Yahoo Paranoids stuartlarsen@yahoo-inc.com

You're correct. The disclosure, as currently written, overstates the severity of the problem.

Still, it's important that this was fixed because it might be possible to get a server to call one of these:

socket.send(number)
socket.ping(number)
socket.pong(number)

especially if the socket server has some echo functionality:

server.on('connection', function (socket) {
  socket.on('message', function (message) {
    message = JSON.parse(message)
    if (message.type === 'echo') {
      socket.send(message.data)
    }
  })
})

socket.send(number) called on the server, will disclose server memory.