FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

src: fix bad logic in uid/gid checks · nodejs/node@53a67ed · GitHub

/ node Public

Commit 53a67ed

Browse files
bnoordhuis
authored and
Myles Borins
committed
src: fix bad logic in uid/gid checks
Pointed out by Coverity. Introduced in commits 3546383 ("process_wrap: avoid leaking memory when throwing due to invalid arguments") and fa4eb47 ("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: #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>
1 parent e6a27a7 commit 53a67ed

3 files changed

Lines changed: 11 additions & 37 deletions

File tree

‎src/process_wrap.cc‎

Lines changed: 4 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -120,25 +120,19 @@ class ProcessWrap : public HandleWrap {
120120
// options.uid
121121
Local<Value> uid_v = js_options->Get(env->uid_string());
122122
if (uid_v->IsInt32()) {
123-
int32_t uid = uid_v->Int32Value();
124-
if (uid & ~((uv_uid_t) ~0)) {
125-
return env->ThrowRangeError("options.uid is out of range");
126-
}
123+
const int32_t uid = uid_v->Int32Value(env->context()).FromJust();
127124
options.flags |= UV_PROCESS_SETUID;
128-
options.uid = (uv_uid_t) uid;
125+
options.uid = static_cast<uv_uid_t>(uid);
129126
} else if (!uid_v->IsUndefined() && !uid_v->IsNull()) {
130127
return env->ThrowTypeError("options.uid should be a number");
131128
}
132129

133130
// options.gid
134131
Local<Value> gid_v = js_options->Get(env->gid_string());
135132
if (gid_v->IsInt32()) {
136-
int32_t gid = gid_v->Int32Value();
137-
if (gid & ~((uv_gid_t) ~0)) {
138-
return env->ThrowRangeError("options.gid is out of range");
139-
}
133+
const int32_t gid = gid_v->Int32Value(env->context()).FromJust();
140134
options.flags |= UV_PROCESS_SETGID;
141-
options.gid = (uv_gid_t) gid;
135+
options.gid = static_cast<uv_gid_t>(gid);
142136
} else if (!gid_v->IsUndefined() && !gid_v->IsNull()) {
143137
return env->ThrowTypeError("options.gid should be a number");
144138
}

‎src/spawn_sync.cc‎

Lines changed: 7 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -729,17 +729,19 @@ int SyncProcessRunner::ParseOptions(Local<Value> js_value) {
729729
}
730730
Local<Value> js_uid = js_options->Get(env()->uid_string());
731731
if (IsSet(js_uid)) {
732-
if (!CheckRange<uv_uid_t>(js_uid))
732+
if (!js_uid->IsInt32())
733733
return UV_EINVAL;
734-
uv_process_options_.uid = static_cast<uv_gid_t>(js_uid->Int32Value());
734+
const int32_t uid = js_uid->Int32Value(env()->context()).FromJust();
735+
uv_process_options_.uid = static_cast<uv_uid_t>(uid);
735736
uv_process_options_.flags |= UV_PROCESS_SETUID;
736737
}
737738

738739
Local<Value> js_gid = js_options->Get(env()->gid_string());
739740
if (IsSet(js_gid)) {
740-
if (!CheckRange<uv_gid_t>(js_gid))
741+
if (!js_gid->IsInt32())
741742
return UV_EINVAL;
742-
uv_process_options_.gid = static_cast<uv_gid_t>(js_gid->Int32Value());
743+
const int32_t gid = js_gid->Int32Value(env()->context()).FromJust();
744+
uv_process_options_.gid = static_cast<uv_gid_t>(gid);
743745
uv_process_options_.flags |= UV_PROCESS_SETGID;
744746
}
745747

@@ -763,7 +765,7 @@ int SyncProcessRunner::ParseOptions(Local<Value> js_value) {
763765

764766
Local<Value> js_max_buffer = js_options->Get(env()->max_buffer_string());
765767
if (IsSet(js_max_buffer)) {
766-
if (!CheckRange<uint32_t>(js_max_buffer))
768+
if (!js_max_buffer->IsUint32())
767769
return UV_EINVAL;
768770
max_buffer_ = js_max_buffer->Uint32Value();
769771
}
@@ -915,27 +917,6 @@ bool SyncProcessRunner::IsSet(Local<Value> value) {
915917
}
916918

917919

918-
template <typename t>
919-
bool SyncProcessRunner::CheckRange(Local<Value> js_value) {
920-
if ((t) -1 > 0) {
921-
// Unsigned range check.
922-
if (!js_value->IsUint32())
923-
return false;
924-
if (js_value->Uint32Value() & ~((t) ~0))
925-
return false;
926-
927-
} else {
928-
// Signed range check.
929-
if (!js_value->IsInt32())
930-
return false;
931-
if (js_value->Int32Value() & ~((t) ~0))
932-
return false;
933-
}
934-
935-
return true;
936-
}
937-
938-
939920
int SyncProcessRunner::CopyJsString(Local<Value> js_value,
940921
const char** target) {
941922
Isolate* isolate = env()->isolate();

‎src/spawn_sync.h‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -173,7 +173,6 @@ class SyncProcessRunner {
173173
inline int AddStdioInheritFD(uint32_t child_fd, int inherit_fd);
174174

175175
static bool IsSet(Local<Value> value);
176-
template <typename t> static bool CheckRange(Local<Value> js_value);
177176
int CopyJsString(Local<Value> js_value, const char** target);
178177
int CopyJsStringArray(Local<Value> js_value, char** target);
179178

0 commit comments

Comments
 (0)
Back | FazBrowse Home | New Git URL