Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Coredump when passing undefined as address from custom lookup function #34812

Closed
joepie91 opened this issue Aug 17, 2020 · 2 comments
Closed
Labels
confirmed-bug Issues with confirmed bugs. dns Issues and PRs related to the dns subsystem. http Issues or PRs related to the http subsystem.

Comments

@joepie91
Copy link
Contributor

What steps will reproduce the bug?

"use strict";

const http = require("http");

http.get({
	host: "google.com:80",
	path: "/",
	lookup: function (host, options, callback) {
		callback(null, undefined, 4);
		// Note the `undefined` here instead of an IP!
	}
});

How often does it reproduce? Is there a required condition?

Completely reproducible.

What is the expected behavior?

Should produce a (catchable/handleable) error indicating that an invalid value was passed from lookup rather than aborting, since this is public API.

What do you see instead?

node[18969]: ../src/tcp_wrap.cc:312:static void node::TCPWrap::Connect(const v8::FunctionCallbackInfo<v8::Value>&, std::function<int(const char*, T*)>) [with T = sockaddr_in]: Assertion `args[1]->IsString()' failed.
 1: 0x9345f8 node::Abort() [node]
 2: 0x934691  [node]
 3: 0x9fda20 void node::TCPWrap::Connect<sockaddr_in>(v8::FunctionCallbackInfo<v8::Value> const&, std::function<int (char const*, sockaddr_in*)>) [node]
 4: 0x9fc95c node::TCPWrap::Connect(v8::FunctionCallbackInfo<v8::Value> const&) [node]
 5: 0xb0e819 v8::internal::FunctionCallbackArguments::Call(v8::internal::CallHandlerInfo) [node]
 6: 0xb0ebd0  [node]
 7: 0xb0fa6a  [node]
 8: 0xb0fcf9 v8::internal::Builtin_HandleApiCall(int, unsigned long*, v8::internal::Isolate*) [node]
 9: 0x12af4f9  [node]
Aborted (core dumped)

Additional information

N/A

@jasnell jasnell added confirmed-bug Issues with confirmed bugs. dns Issues and PRs related to the dns subsystem. http Issues or PRs related to the http subsystem. labels Aug 17, 2020
@jasnell
Copy link
Member

jasnell commented Aug 17, 2020

Confirmed, able to easily reproduce

@joepie91
Copy link
Contributor Author

Thanks! 🎉

BethGriggs pushed a commit that referenced this issue Aug 20, 2020
This commit adds validation to the IP address returned by the
net module's custom DNS lookup() function.

PR-URL: #34813
Fixes: #34812
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Yongsheng Zhang <[email protected]>
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Michaël Zasso <[email protected]>
Reviewed-By: Ricky Zhou <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
BethGriggs pushed a commit that referenced this issue Aug 20, 2020
This commit adds validation to the IP address returned by the
net module's custom DNS lookup() function.

PR-URL: #34813
Fixes: #34812
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Yongsheng Zhang <[email protected]>
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Michaël Zasso <[email protected]>
Reviewed-By: Ricky Zhou <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
addaleax pushed a commit that referenced this issue Sep 22, 2020
This commit adds validation to the IP address returned by the
net module's custom DNS lookup() function.

PR-URL: #34813
Fixes: #34812
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Yongsheng Zhang <[email protected]>
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Michaël Zasso <[email protected]>
Reviewed-By: Ricky Zhou <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
addaleax pushed a commit that referenced this issue Sep 22, 2020
This commit adds validation to the IP address returned by the
net module's custom DNS lookup() function.

PR-URL: #34813
Fixes: #34812
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Yongsheng Zhang <[email protected]>
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Michaël Zasso <[email protected]>
Reviewed-By: Ricky Zhou <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
confirmed-bug Issues with confirmed bugs. dns Issues and PRs related to the dns subsystem. http Issues or PRs related to the http subsystem.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants