src: avoid dereference without existence check #14591
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
nodejs-github-bot
added
c++
4 tasks
jasnell
approved these changes
Aug 2, 2017
cjihrig
approved these changes
Aug 2, 2017
tniessen
approved these changes
Aug 3, 2017
Currently the URL API is only used from the JS binding, which always initializes `base` regardless of `has_base`. Therefore, there is no actual security risk right now, but would be had we made other C++ parts of Node.js use this API. Refs: nodejs#14369 (comment)
|
Landed w/o a CI run... force-pushed to remove it. |
|
CI failures are known flakies, will land this soon. |
|
Landed in e96ca62. |
TimothyGu
added a commit
that referenced
this pull request
Aug 6, 2017
Currently the URL API is only used from the JS binding, which always initializes `base` regardless of `has_base`. Therefore, there is no actual security risk right now, but would be had we made other C++ parts of Node.js use this API. An earlier version of this patch was created by Bradley Farias <[email protected]>. PR-URL: #14591 Refs: #14369 (comment) Reviewed-By: James M Snell <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Tobias Nießen <[email protected]>
addaleax
pushed a commit
that referenced
this pull request
Aug 7, 2017
Currently the URL API is only used from the JS binding, which always initializes `base` regardless of `has_base`. Therefore, there is no actual security risk right now, but would be had we made other C++ parts of Node.js use this API. An earlier version of this patch was created by Bradley Farias <[email protected]>. PR-URL: #14591 Refs: #14369 (comment) Reviewed-By: James M Snell <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Tobias Nießen <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Currently the URL API is only used from the JS binding, which always initializes
baseregardless ofhas_base. Therefore, there is no actual security risk right now, but there would be had we made other C++ parts of Node.js use this API.Because of that, I was not able to add any JS tests, instead resorting to C++. However, the magic of C++ compilers makes a reliable test impossible, as the added test only sometimes fails before this PR, even though when it does fail it is a NULL pointer dereference.
Refs: #14369 (comment)
Checklist
make -j4 test(UNIX), orvcbuild test(Windows) passesAffected core subsystem(s)
url