{"action":"created","issue":{"url":"https://api.github.com/repos/nodejs/node/issues/4514","labels_url":"https://api.github.com/repos/nodejs/node/issues/4514/labels{/name}","comments_url":"https://api.github.com/repos/nodejs/node/issues/4514/comments","events_url":"https://api.github.com/repos/nodejs/node/issues/4514/events","html_url":"https://github.com/nodejs/node/pull/4514","id":124597838,"number":4514,"title":"buffer: throw if both length and enc are passed","user":{"login":"mafintosh","id":376661,"avatar_url":"https://avatars.githubusercontent.com/u/376661?v=3","gravatar_id":"","url":"https://api.github.com/users/mafintosh","html_url":"https://github.com/mafintosh","followers_url":"https://api.github.com/users/mafintosh/followers","following_url":"https://api.github.com/users/mafintosh/following{/other_user}","gists_url":"https://api.github.com/users/mafintosh/gists{/gist_id}","starred_url":"https://api.github.com/users/mafintosh/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/mafintosh/subscriptions","organizations_url":"https://api.github.com/users/mafintosh/orgs","repos_url":"https://api.github.com/users/mafintosh/repos","events_url":"https://api.github.com/users/mafintosh/events{/privacy}","received_events_url":"https://api.github.com/users/mafintosh/received_events","type":"User","site_admin":false},"labels":[{"url":"https://api.github.com/repos/nodejs/node/labels/buffer","name":"buffer","color":"f7c6c7"},{"url":"https://api.github.com/repos/nodejs/node/labels/semver-major","name":"semver-major","color":"fbca04"}],"state":"open","locked":false,"assignee":null,"milestone":null,"comments":7,"created_at":"2016-01-02T16:24:11Z","updated_at":"2016-01-03T01:17:09Z","closed_at":null,"pull_request":{"url":"https://api.github.com/repos/nodejs/node/pulls/4514","html_url":"https://github.com/nodejs/node/pull/4514","diff_url":"https://github.com/nodejs/node/pull/4514.diff","patch_url":"https://github.com/nodejs/node/pull/4514.patch"},"body":"We just fixed a security issue in the [bittorrent-dht](https://github.com/feross/bittorrent-dht) module that allowed a peer to craft a specific message that would make a remote peer disclose memory.\r\n\r\nThe root cause of the issue was this snippet:\r\n\r\n``` js\r\nvar nodeId = new Buffer(nodeIdString, 'hex')\r\n```\r\n\r\nThe `nodeIdString` was received over the network and by sending back a number instead of a string the `new Buffer` constructor would return a non-zeroed out buffer. We fixed it in our code base by type checking the argument (which we of course should have done from the beginning) but it would have been easier to catch if node had thrown an exception.\r\n\r\nI'm sure that are other modules out there that have the same kind of vulnerability that would benefit from this PR as well."},"comment":{"url":"https://api.github.com/repos/nodejs/node/issues/comments/168448527","html_url":"https://github.com/nodejs/node/pull/4514#issuecomment-168448527","issue_url":"https://api.github.com/repos/nodejs/node/issues/4514","id":168448527,"user":{"login":"rvagg","id":495647,"avatar_url":"https://avatars.githubusercontent.com/u/495647?v=3","gravatar_id":"","url":"https://api.github.com/users/rvagg","html_url":"https://github.com/rvagg","followers_url":"https://api.github.com/users/rvagg/followers","following_url":"https://api.github.com/users/rvagg/following{/other_user}","gists_url":"https://api.github.com/users/rvagg/gists{/gist_id}","starred_url":"https://api.github.com/users/rvagg/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/rvagg/subscriptions","organizations_url":"https://api.github.com/users/rvagg/orgs","repos_url":"https://api.github.com/users/rvagg/repos","events_url":"https://api.github.com/users/rvagg/events{/privacy}","received_events_url":"https://api.github.com/users/rvagg/received_events","type":"User","site_admin":false},"created_at":"2016-01-03T01:17:09Z","updated_at":"2016-01-03T01:17:09Z","body":"My current inclination is to LGTM this and to endorse @vkurchatkin's `semver-major` label rather than treat this as a bug given that it's likely being used in the wild in places where there may not be security concerns (e.g. in situations where you have full control over the source of the inputs).\r\n\r\nIf we go with `semver-major` I'd vote for adding a deprecation warning in v5 and v4 when you supply an `enc` as well as a `'number'` as the first argument."}}