diff options
author | Tobias Nießen <tniessen@tnie.de> | 2022-08-23 01:03:36 +0300 |
---|---|---|
committer | GitHub <noreply@github.com> | 2022-08-23 01:03:36 +0300 |
commit | fcd31c5110d510fe2fe94a93eb3eff77ab4b4e43 (patch) | |
tree | 811e52bd3c1a1ab2cf9c0b6309f322b455706ab5 /src | |
parent | a5671e266241b059565a7575683ff1445c0dc6fa (diff) |
src: fix multiple format string bugs
The THROW_ERR_* functions interpret the first argument as a printf-like
format string, which is problematic when it contains unsanitized user
input. This typically happens when a printf-like function is used to
produce the error message, which is then passed to a THROW_ERR_*
function, which again interprets the error message as a format string.
Fix such occurrences by properly formatting error messages using static
format strings only, and in a single step.
PR-URL: https://github.com/nodejs/node/pull/44314
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Darshan Sen <raisinten@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Zeyu "Alex" Yang <himself65@outlook.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Diffstat (limited to 'src')
-rw-r--r-- | src/crypto/crypto_context.cc | 4 | ||||
-rw-r--r-- | src/crypto/crypto_keygen.cc | 8 | ||||
-rw-r--r-- | src/node_binding.cc | 47 |
3 files changed, 23 insertions, 36 deletions
diff --git a/src/crypto/crypto_context.cc b/src/crypto/crypto_context.cc index f8ea6f9acca..7eab9de37cb 100644 --- a/src/crypto/crypto_context.cc +++ b/src/crypto/crypto_context.cc @@ -501,8 +501,8 @@ void SecureContext::Init(const FunctionCallbackInfo<Value>& args) { max_version = TLS1_2_VERSION; method = TLS_client_method(); } else { - const std::string msg("Unknown method: "); - THROW_ERR_TLS_INVALID_PROTOCOL_METHOD(env, (msg + * sslmethod).c_str()); + THROW_ERR_TLS_INVALID_PROTOCOL_METHOD( + env, "Unknown method: %s", *sslmethod); return; } } diff --git a/src/crypto/crypto_keygen.cc b/src/crypto/crypto_keygen.cc index e4e9c227458..e8f55806de6 100644 --- a/src/crypto/crypto_keygen.cc +++ b/src/crypto/crypto_keygen.cc @@ -68,11 +68,9 @@ Maybe<bool> SecretKeyGenTraits::AdditionalConfig( params->length = static_cast<size_t>( std::trunc(args[*offset].As<Uint32>()->Value() / CHAR_BIT)); if (params->length > INT_MAX) { - const std::string msg{ - SPrintF("length must be less than or equal to %s bits", - static_cast<uint64_t>(INT_MAX) * CHAR_BIT) - }; - THROW_ERR_OUT_OF_RANGE(env, msg.c_str()); + THROW_ERR_OUT_OF_RANGE(env, + "length must be less than or equal to %u bits", + static_cast<uint64_t>(INT_MAX) * CHAR_BIT); return Nothing<bool>(); } *offset += 1; diff --git a/src/node_binding.cc b/src/node_binding.cc index 60eca5c9fa5..fa67a45386e 100644 --- a/src/node_binding.cc +++ b/src/node_binding.cc @@ -459,7 +459,7 @@ void DLOpen(const FunctionCallbackInfo<Value>& args) { // Windows needs to add the filename into the error message errmsg += *filename; #endif // _WIN32 - THROW_ERR_DLOPEN_FAILED(env, errmsg.c_str()); + THROW_ERR_DLOPEN_FAILED(env, "%s", errmsg.c_str()); return false; } @@ -484,12 +484,8 @@ void DLOpen(const FunctionCallbackInfo<Value>& args) { mp = dlib->GetSavedModuleFromGlobalHandleMap(); if (mp == nullptr || mp->nm_context_register_func == nullptr) { dlib->Close(); - char errmsg[1024]; - snprintf(errmsg, - sizeof(errmsg), - "Module did not self-register: '%s'.", - *filename); - THROW_ERR_DLOPEN_FAILED(env, errmsg); + THROW_ERR_DLOPEN_FAILED( + env, "Module did not self-register: '%s'.", *filename); return false; } } @@ -504,23 +500,22 @@ void DLOpen(const FunctionCallbackInfo<Value>& args) { callback(exports, module, context); return true; } - char errmsg[1024]; - snprintf(errmsg, - sizeof(errmsg), - "The module '%s'" - "\nwas compiled against a different Node.js version using" - "\nNODE_MODULE_VERSION %d. This version of Node.js requires" - "\nNODE_MODULE_VERSION %d. Please try re-compiling or " - "re-installing\nthe module (for instance, using `npm rebuild` " - "or `npm install`).", - *filename, - mp->nm_version, - NODE_MODULE_VERSION); + const int actual_nm_version = mp->nm_version; // NOTE: `mp` is allocated inside of the shared library's memory, calling // `dlclose` will deallocate it dlib->Close(); - THROW_ERR_DLOPEN_FAILED(env, errmsg); + THROW_ERR_DLOPEN_FAILED( + env, + "The module '%s'" + "\nwas compiled against a different Node.js version using" + "\nNODE_MODULE_VERSION %d. This version of Node.js requires" + "\nNODE_MODULE_VERSION %d. Please try re-compiling or " + "re-installing\nthe module (for instance, using `npm rebuild` " + "or `npm install`).", + *filename, + actual_nm_version, + NODE_MODULE_VERSION); return false; } CHECK_EQ(mp->nm_flags & NM_F_BUILTIN, 0); @@ -600,9 +595,7 @@ void GetInternalBinding(const FunctionCallbackInfo<Value>& args) { builtins::BuiltinLoader::GetConfigString(env->isolate())) .FromJust()); } else { - char errmsg[1024]; - snprintf(errmsg, sizeof(errmsg), "No such module: %s", *module_v); - return THROW_ERR_INVALID_MODULE(env, errmsg); + return THROW_ERR_INVALID_MODULE(env, "No such module: %s", *module_v); } args.GetReturnValue().Set(exports); @@ -632,12 +625,8 @@ void GetLinkedBinding(const FunctionCallbackInfo<Value>& args) { mod = FindModule(modlist_linked, name, NM_F_LINKED); if (mod == nullptr) { - char errmsg[1024]; - snprintf(errmsg, - sizeof(errmsg), - "No such module was linked: %s", - *module_name_v); - return THROW_ERR_INVALID_MODULE(env, errmsg); + return THROW_ERR_INVALID_MODULE( + env, "No such module was linked: %s", *module_name_v); } Local<Object> module = Object::New(env->isolate()); |