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 <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Michael Dawson <[email protected]>

Author: bnoordhuis

src/process_wrap.cc

@@ -120,25 +120,19 @@ 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");
- }
+constint32_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);
  } elseif (!uid_v->IsUndefined() && !uid_v->IsNull()){
 return env->ThrowTypeError("options.uid should be a number");
  }
 
 // 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");
- }
+constint32_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);
  } elseif (!gid_v->IsUndefined() && !gid_v->IsNull()){
 return env->ThrowTypeError("options.gid should be a number");
  }

src/spawn_sync.cc

@@ -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());
+constint32_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());
+constint32_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>
-boolSyncProcessRunner::CheckRange(Local<Value> js_value){
-if ((t) -1 > 0){
-// Unsigned range check.
-if (!js_value->IsUint32())
-returnfalse;
-if (js_value->Uint32Value() & ~((t) ~0))
-returnfalse;
-
- } else{
-// Signed range check.
-if (!js_value->IsInt32())
-returnfalse;
-if (js_value->Int32Value() & ~((t) ~0))
-returnfalse;
- }
-
-returntrue;
-}
-
-
 intSyncProcessRunner::CopyJsString(Local<Value> js_value,
 constchar** target){
  Isolate* isolate = env()->isolate();

src/spawn_sync.h

@@ -173,7 +173,6 @@ class SyncProcessRunner{
 inlineintAddStdioInheritFD(uint32_t child_fd, int inherit_fd);
 
 staticboolIsSet(Local<Value> value);
-template <typename t> staticboolCheckRange(Local<Value> js_value);
 intCopyJsString(Local<Value> js_value, constchar** target);
 intCopyJsStringArray(Local<Value> js_value, char** target);