-
Notifications
You must be signed in to change notification settings - Fork 29.6k
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
doc: fix coverity report #42663
doc: fix coverity report #42663
Conversation
Fix coverity report about possibly dereferencing a null. If the the buffer.data != nullptr check indicates that the buffer was null, then relying on the value in buffer_size is no longer safe. The later call to uv_pipe_getpeername depends on the buffer_size being correct to avoid deferencing buffer.data if it is not big enough. Signed-off-by: Michael Dawson <mdawson@devrus.com>
Report from Coverity // First call to get required buffer size.
93 rc = uv_pipe_getsockname(&handle->pipe, buffer.data, &buffer_size);
1. Condition rc == UV_ENOBUFS, taking true branch.
94 if (rc == UV_ENOBUFS) {
95 buffer = MallocedBuffer<char>(buffer_size);
2. Condition buffer.data != NULL, taking false branch.
3. var_compare_op: Comparing buffer.data to null implies that buffer.data might be null.
96 if (buffer.data != nullptr) {
97 rc = uv_pipe_getsockname(&handle->pipe, buffer.data, &buffer_size);
98 }
99 }
4. Condition rc == 0, taking false branch.
100 if (rc == 0 && buffer_size != 0 && buffer.data != nullptr) {
101 writer->json_keyvalue("localEndpoint", buffer.data);
102 } else {
103 writer->json_keyvalue("localEndpoint", null);
104 }
105
106 // First call to get required buffer size.
CID 239713 (#1 of 1): Dereference after null check (FORWARD_NULL)
5. var_deref_model: Passing null pointer buffer.data to uv_pipe_getpeername, which dereferences it.
107 rc = uv_pipe_getpeername(&handle->pipe, buffer.data, &buffer_size);
108 if (rc == UV_ENOBUFS) {
109 buffer = MallocedBuffer<char>(buffer_size);
110 if (buffer.data != nullptr) {
111 rc = uv_pipe_getpeername(&handle->pipe, buffer.data, &buffer_size);
112 }
113 } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This doesn't look like the correct fix because buffer.data can't be null here. Also, should we use src
as the subsystem instead of doc
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
@mhdawson wdyt about?
Are you planning to change it while landing this? |
Good point, I must have had doc on my mind, will change while landing. |
CI run looks to be complete (https://ci.nodejs.org/job/node-test-pull-request/43464/) even though what's shown on the PR shows a job still running. Will land. |
Fix coverity report about possibly dereferencing a null. If the the buffer.data != nullptr check indicates that the buffer was null, then relying on the value in buffer_size is no longer safe. The later call to uv_pipe_getpeername depends on the buffer_size being correct to avoid deferencing buffer.data if it is not big enough. Signed-off-by: Michael Dawson <mdawson@devrus.com> PR-URL: #42663 Reviewed-By: Darshan Sen <raisinten@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
Landed in 3026ca0 |
Fix coverity report about possibly dereferencing a null. If the the buffer.data != nullptr check indicates that the buffer was null, then relying on the value in buffer_size is no longer safe. The later call to uv_pipe_getpeername depends on the buffer_size being correct to avoid deferencing buffer.data if it is not big enough. Signed-off-by: Michael Dawson <mdawson@devrus.com> PR-URL: nodejs#42663 Reviewed-By: Darshan Sen <raisinten@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
Fix coverity report about possibly dereferencing a null. If the the buffer.data != nullptr check indicates that the buffer was null, then relying on the value in buffer_size is no longer safe. The later call to uv_pipe_getpeername depends on the buffer_size being correct to avoid deferencing buffer.data if it is not big enough. Signed-off-by: Michael Dawson <mdawson@devrus.com> PR-URL: nodejs#42663 Reviewed-By: Darshan Sen <raisinten@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
Fix coverity report about possibly dereferencing a null. If the the buffer.data != nullptr check indicates that the buffer was null, then relying on the value in buffer_size is no longer safe. The later call to uv_pipe_getpeername depends on the buffer_size being correct to avoid deferencing buffer.data if it is not big enough. Signed-off-by: Michael Dawson <mdawson@devrus.com> PR-URL: #42663 Reviewed-By: Darshan Sen <raisinten@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
Fix coverity report about possibly dereferencing a null. If the the buffer.data != nullptr check indicates that the buffer was null, then relying on the value in buffer_size is no longer safe. The later call to uv_pipe_getpeername depends on the buffer_size being correct to avoid deferencing buffer.data if it is not big enough. Signed-off-by: Michael Dawson <mdawson@devrus.com> PR-URL: #42663 Reviewed-By: Darshan Sen <raisinten@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
Fix coverity report about possibly dereferencing a null. If the the buffer.data != nullptr check indicates that the buffer was null, then relying on the value in buffer_size is no longer safe. The later call to uv_pipe_getpeername depends on the buffer_size being correct to avoid deferencing buffer.data if it is not big enough. Signed-off-by: Michael Dawson <mdawson@devrus.com> PR-URL: #42663 Reviewed-By: Darshan Sen <raisinten@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
Fix coverity report about possibly dereferencing a null. If the the buffer.data != nullptr check indicates that the buffer was null, then relying on the value in buffer_size is no longer safe. The later call to uv_pipe_getpeername depends on the buffer_size being correct to avoid deferencing buffer.data if it is not big enough. Signed-off-by: Michael Dawson <mdawson@devrus.com> PR-URL: #42663 Reviewed-By: Darshan Sen <raisinten@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
Fix coverity report about possibly dereferencing a null. If the the buffer.data != nullptr check indicates that the buffer was null, then relying on the value in buffer_size is no longer safe. The later call to uv_pipe_getpeername depends on the buffer_size being correct to avoid deferencing buffer.data if it is not big enough. Signed-off-by: Michael Dawson <mdawson@devrus.com> PR-URL: nodejs/node#42663 Reviewed-By: Darshan Sen <raisinten@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
Fix coverity report about possibly dereferencing
a null. If the the buffer.data != nullptr
check indicates that the buffer was null, then
relying on the value in buffer_size is no longer
safe. The later call to uv_pipe_getpeername
depends on the buffer_size being correct to
avoid deferencing buffer.data if it is not
big enough.
Signed-off-by: Michael Dawson mdawson@devrus.com