Fix remote memory disclosure with multipart attachments

[feross]

If the node process makes a request with a multipart attachment, and the type of the body option is a Number, then that many bytes of uninitialized memory will be sent in the body of the request. Here's an example:

request({
  method: 'POST',
  uri: 'http://localhost:8000',
  multipart: [{ body: 1000 }]
}

And here's a more complete reproducible example:

var http = require('http')
var request = require('request')

http.createServer(function (req, res) {
  var data = ''
  req.setEncoding('utf8')
  req.on('data', function (chunk) {
    console.log('data')
    data += chunk
  })
  req.on('end', function () {
    // this will print uninitialized memory from the client
    console.log('Client sent:\n', data)
  })
  res.end()
}).listen(8000)

request({
  method: 'POST',
  uri: 'http://localhost:8000',
  multipart: [{ body: 1000 }]
},
function (err, res, body) {
  if (err) return console.error('upload failed:', err)
  console.log('sent')
})

This PR fixes the issue by coercing the type of the body option to a string. (An alternate solution would be to throw an exception in this case.)

Further reading:

• Buffer(number) is unsafe nodejs/node#4660
• Buffer.from/Buffer.alloc/Buffer.zalloc/Buffer() soft-deprecate nodejs/node-eps#4

[feross]

For the record, I'm sharing this here for the first time since this doesn't seem particularly easy to exploit. That said, it's still probably a good idea to release a fix in a timely manner.

[simov]

Thanks I've added your commit here #2022 + a test case.

[ChALkeR]

@feross

For the record, I'm sharing this here for the first time since this doesn't seem particularly easy to exploit. That said, it's still probably a good idea to release a fix in a timely manner.

It still looks like a much better idea to send those over emails. Can I have a chat with you on IRC or Gitter?

[ChALkeR]

@feross I know of at least one npm module that reads config from a JSON file and sends a multipart request using typed value from that config as the raw body entry of some multipart data to a remote server, using request. And I think that this is much likely to be done in actual setups (aka apps), not only npm modules.

[grnd]

Vulnerable range: <2.68.0 >2.2.5 (total of 77 releases)
There is one version in that range that's not exploitable, 2.47.0 (throws TypeError).
While<=2.2.5 is not vulnerable because part's body is wrapped with '\r\n' just before it is passed to Buffer.

      self.body += '\r\n' + body + '\r\n'
    })
    self.body += '--frontier--'
  }

  if (self.body) {
    if (!Buffer.isBuffer(self.body)) {
      self.body = new Buffer(self.body)
    }

[evilaliv3]

I just found out that relevant and widespread softwares are still impacted:

• Update request dependency to version ≥ 2.74.0 nodejs/node-gyp#1000
• Update request to 2.74.0 mapbox/node-pre-gyp#232
• Update request to 2.74.0 yeoman/insight#48
• Update request to 2.74.0 FGRibreau/node-request-retry#37
• Request dependency <=2.68 opens to potential memory exposure vulnerability angular/angular.js#14961
• Request dependency <=2.68 opens to potential memory exposure vulnerability angular/angular#10352
• Request dependency <=2.68 opens to potential memory exposure vulnerability fsevents/fsevents#145
• Update node-request-retry to latest (still not available release) axemclion/grunt-saucelabs#214

I think that it should be important to inform better the users around issues like this.

[ChALkeR]

@evilaliv3 There are about 14000 of those. Ah, sorry, forgot the lower limit. About 10400.