1berry/nodejs
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

src: fix bad logic in uid/gid checks

Browse Source 
Pointed out by Coverity.  Introduced in commits 3546383c ("process_wrap:
avoid leaking memory when throwing due to invalid arguments") and
fa4eb47c ("bindings: add spawn_sync bindings").

The return statements inside the if blocks were dead code because their
guard conditions always evaluated to false.  Remove them.

PR-URL: https://github.com/nodejs/node/pull/7374
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
This commit is contained in:
Ben Noordhuis 2016-06-22 21:57:13 +02:00
parent c50e192204
commit ed3d8b13ee
3 changed files with 11 additions and 37 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

14
src/process_wrap.cc
View File

@ -123,12 +123,9 @@ class ProcessWrap : public HandleWrap {
// options.uid
Local<Value> uid_v = js_options->Get(env->uid_string());
if (uid_v->IsInt32()) {
int32_t uid = uid_v->Int32Value();
if (uid & ~((uv_uid_t) ~0)) {
return env->ThrowRangeError("options.uid is out of range");
}
const int32_t uid = uid_v->Int32Value(env->context()).FromJust();
options.flags |= UV_PROCESS_SETUID;
options.uid = (uv_uid_t) uid;
options.uid = static_cast<uv_uid_t>(uid);
} else if (!uid_v->IsUndefined() && !uid_v->IsNull()) {
return env->ThrowTypeError("options.uid should be a number");
}
@ -136,12 +133,9 @@ class ProcessWrap : public HandleWrap {
// options.gid
Local<Value> gid_v = js_options->Get(env->gid_string());
if (gid_v->IsInt32()) {
int32_t gid = gid_v->Int32Value();
if (gid & ~((uv_gid_t) ~0)) {
return env->ThrowRangeError("options.gid is out of range");
}
const int32_t gid = gid_v->Int32Value(env->context()).FromJust();
options.flags |= UV_PROCESS_SETGID;
options.gid = (uv_gid_t) gid;
options.gid = static_cast<uv_gid_t>(gid);
} else if (!gid_v->IsUndefined() && !gid_v->IsNull()) {
return env->ThrowTypeError("options.gid should be a number");
}

33
src/spawn_sync.cc
View File

@ -729,17 +729,19 @@ int SyncProcessRunner::ParseOptions(Local<Value> js_value) {
}
Local<Value> js_uid = js_options->Get(env()->uid_string());
if (IsSet(js_uid)) {
if (!CheckRange<uv_uid_t>(js_uid))
if (!js_uid->IsInt32())
return UV_EINVAL;
uv_process_options_.uid = static_cast<uv_gid_t>(js_uid->Int32Value());
const int32_t uid = js_uid->Int32Value(env()->context()).FromJust();
uv_process_options_.uid = static_cast<uv_uid_t>(uid);
uv_process_options_.flags |= UV_PROCESS_SETUID;
}
Local<Value> js_gid = js_options->Get(env()->gid_string());
if (IsSet(js_gid)) {
if (!CheckRange<uv_gid_t>(js_gid))
if (!js_gid->IsInt32())
return UV_EINVAL;
uv_process_options_.gid = static_cast<uv_gid_t>(js_gid->Int32Value());
const int32_t gid = js_gid->Int32Value(env()->context()).FromJust();
uv_process_options_.gid = static_cast<uv_gid_t>(gid);
uv_process_options_.flags |= UV_PROCESS_SETGID;
}
@ -763,7 +765,7 @@ int SyncProcessRunner::ParseOptions(Local<Value> js_value) {
Local<Value> js_max_buffer = js_options->Get(env()->max_buffer_string());
if (IsSet(js_max_buffer)) {
if (!CheckRange<uint32_t>(js_max_buffer))
if (!js_max_buffer->IsUint32())
return UV_EINVAL;
max_buffer_ = js_max_buffer->Uint32Value();
}
@ -915,27 +917,6 @@ bool SyncProcessRunner::IsSet(Local<Value> value) {
}
template <typename t>
bool SyncProcessRunner::CheckRange(Local<Value> js_value) {
if ((t) -1 > 0) {
// Unsigned range check.
if (!js_value->IsUint32())
return false;
if (js_value->Uint32Value() & ~((t) ~0))
return false;
} else {
// Signed range check.
if (!js_value->IsInt32())
return false;
if (js_value->Int32Value() & ~((t) ~0))
return false;
}
return true;
}
int SyncProcessRunner::CopyJsString(Local<Value> js_value,
const char** target) {
Isolate* isolate = env()->isolate();

1
src/spawn_sync.h
View File

@ -175,7 +175,6 @@ class SyncProcessRunner {
inline int AddStdioInheritFD(uint32_t child_fd, int inherit_fd);
static bool IsSet(Local<Value> value);
template <typename t> static bool CheckRange(Local<Value> js_value);
int CopyJsString(Local<Value> js_value, const char** target);
int CopyJsStringArray(Local<Value> js_value, char** target);