Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Commit 53a67ed

Browse filesBrowse files
bnoordhuisMyles Borins
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
Copy full SHA for 53a67ed

File tree

Expand file treeCollapse file tree

3 files changed

+11
-37
lines changed
Open diff view settings
Filter options
Expand file treeCollapse file tree

3 files changed

+11
-37
lines changed
Open diff view settings
Collapse file

‎src/process_wrap.cc‎

Copy file name to clipboardExpand all lines: src/process_wrap.cc
+4-10Lines 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
}
Collapse file

‎src/spawn_sync.cc‎

Copy file name to clipboardExpand all lines: src/spawn_sync.cc
+7-26Lines 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();
Collapse file

‎src/spawn_sync.h‎

Copy file name to clipboardExpand all lines: src/spawn_sync.h
-1Lines 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 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.