From b30546edb334530f682ab1e7dec786fcb539015d Mon Sep 17 00:00:00 2001 From: Raymond Zhao <7199958+rzhao271@users.noreply.github.com> Date: Mon, 31 Jul 2023 16:39:22 -0700 Subject: [PATCH 1/5] Add compiler warnings --- binding.gyp | 9 ++++++++- src/win32/kerberos_win32.cc | 6 +++--- 2 files changed, 11 insertions(+), 4 deletions(-) diff --git a/binding.gyp b/binding.gyp index f4954ea5..7ed55413 100644 --- a/binding.gyp +++ b/binding.gyp @@ -15,7 +15,14 @@ 'cflags!': [ '-fno-exceptions' ], 'cflags_cc!': [ '-fno-exceptions' ], 'msvs_settings': { - 'VCCLCompilerTool': { 'ExceptionHandling': 1 }, + 'VCCLCompilerTool': { + 'ExceptionHandling': 1, + 'AdditionalOptions': [ + '/w34244', + '/w34267', + '/ZH:SHA_256' + ] + }, }, 'conditions': [ ['OS=="mac"', { diff --git a/src/win32/kerberos_win32.cc b/src/win32/kerberos_win32.cc index c37b7eb6..7306b4a6 100644 --- a/src/win32/kerberos_win32.cc +++ b/src/win32/kerberos_win32.cc @@ -88,7 +88,7 @@ void KerberosClient::WrapData(const CallbackInfo& info) { KerberosWorker::Run(callback, "kerberos:ClientWrap", [=](KerberosWorker::SetOnFinishedHandler onFinished) { sspi_result result = auth_sspi_client_wrap( - state.get(), (SEC_CHAR*)challenge.c_str(), (SEC_CHAR*)user.c_str(), user.length(), protect); + state.get(), (SEC_CHAR*)challenge.c_str(), (SEC_CHAR*)user.c_str(), (ULONG)user.length(), protect); return onFinished([=](KerberosWorker* worker) { Napi::Env env = worker->Env(); @@ -131,8 +131,8 @@ void InitializeClient(const CallbackInfo& info) { KerberosWorker::Run(callback, "kerberos:InitializeClient", [=](KerberosWorker::SetOnFinishedHandler onFinished) { auto client_state = std::make_shared(); sspi_result result = auth_sspi_client_init( - (WCHAR*)service.c_str(), gss_flags, (WCHAR*)user.c_str(), user.length(), - (WCHAR*)domain.c_str(), domain.length(), (WCHAR*)password.c_str(), password.length(), + (WCHAR*)service.c_str(), gss_flags, (WCHAR*)user.c_str(), (ULONG)user.length(), + (WCHAR*)domain.c_str(), (ULONG)domain.length(), (WCHAR*)password.c_str(), (ULONG)password.length(), (WCHAR*)mech_oid.c_str(), client_state.get()); return onFinished([=](KerberosWorker* worker) { From 92333b0da7fd7552c12e75b4092e4e776eada056 Mon Sep 17 00:00:00 2001 From: Raymond Zhao <7199958+rzhao271@users.noreply.github.com> Date: Wed, 2 Aug 2023 12:01:26 -0700 Subject: [PATCH 2/5] Add parameter checks --- src/win32/kerberos_win32.cc | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/src/win32/kerberos_win32.cc b/src/win32/kerberos_win32.cc index 7306b4a6..115c28d7 100644 --- a/src/win32/kerberos_win32.cc +++ b/src/win32/kerberos_win32.cc @@ -78,6 +78,10 @@ void KerberosClient::UnwrapData(const CallbackInfo& info) { }); } +static bool isStringTooLong(const std::string& str) { + return str.length() >= ULONG_MAX; +} + void KerberosClient::WrapData(const CallbackInfo& info) { auto state = this->state(); std::string challenge = info[0].ToString(); @@ -86,6 +90,11 @@ void KerberosClient::WrapData(const CallbackInfo& info) { std::string user = ToStringWithNonStringAsEmpty(options["user"]); int protect = 0; // NOTE: this should be an option + if (isStringTooLong(user)) { + throw Error::New(info.Env(), "User name is too long"); + return; + } + KerberosWorker::Run(callback, "kerberos:ClientWrap", [=](KerberosWorker::SetOnFinishedHandler onFinished) { sspi_result result = auth_sspi_client_wrap( state.get(), (SEC_CHAR*)challenge.c_str(), (SEC_CHAR*)user.c_str(), (ULONG)user.length(), protect); @@ -119,6 +128,20 @@ void InitializeClient(const CallbackInfo& info) { std::wstring user = ToWStringWithNonStringAsEmpty(options["user"]); std::wstring domain = ToWStringWithNonStringAsEmpty(options["domain"]); std::wstring password = ToWStringWithNonStringAsEmpty(options["password"]); + + if (isStringTooLong(user)) { + throw Error::New(info.Env(), "User name is too long"); + return; + } + if (isStringTooLong(domain)) { + throw Error::New(info.Env(), "Domain is too long"); + return; + } + if (isStringTooLong(password)) { + throw Error::New(info.Env(), "Password is too long"); + return; + } + Value flags_v = options["flags"]; ULONG gss_flags = flags_v.IsNumber() ? flags_v.As().Uint32Value() : GSS_C_MUTUAL_FLAG|GSS_C_SEQUENCE_FLAG; Value mech_oid_v = options["mechOID"]; From e7913b7fad3b1e157ede2dcd2ab89fb050181130 Mon Sep 17 00:00:00 2001 From: Raymond Zhao <7199958+rzhao271@users.noreply.github.com> Date: Wed, 2 Aug 2023 14:11:07 -0700 Subject: [PATCH 3/5] Use size instead of length --- src/win32/kerberos_win32.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/win32/kerberos_win32.cc b/src/win32/kerberos_win32.cc index 115c28d7..c8aae607 100644 --- a/src/win32/kerberos_win32.cc +++ b/src/win32/kerberos_win32.cc @@ -79,7 +79,7 @@ void KerberosClient::UnwrapData(const CallbackInfo& info) { } static bool isStringTooLong(const std::string& str) { - return str.length() >= ULONG_MAX; + return str.size() >= ULONG_MAX; } void KerberosClient::WrapData(const CallbackInfo& info) { From d8a9afbe36b507594fb56040f5593d04e31aac67 Mon Sep 17 00:00:00 2001 From: Raymond Zhao <7199958+rzhao271@users.noreply.github.com> Date: Thu, 3 Aug 2023 09:17:55 -0700 Subject: [PATCH 4/5] Remove extra return statements --- src/win32/kerberos_win32.cc | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/src/win32/kerberos_win32.cc b/src/win32/kerberos_win32.cc index c8aae607..9ed902e2 100644 --- a/src/win32/kerberos_win32.cc +++ b/src/win32/kerberos_win32.cc @@ -79,7 +79,7 @@ void KerberosClient::UnwrapData(const CallbackInfo& info) { } static bool isStringTooLong(const std::string& str) { - return str.size() >= ULONG_MAX; + return str.length() >= ULONG_MAX; } void KerberosClient::WrapData(const CallbackInfo& info) { @@ -92,7 +92,6 @@ void KerberosClient::WrapData(const CallbackInfo& info) { if (isStringTooLong(user)) { throw Error::New(info.Env(), "User name is too long"); - return; } KerberosWorker::Run(callback, "kerberos:ClientWrap", [=](KerberosWorker::SetOnFinishedHandler onFinished) { @@ -131,15 +130,12 @@ void InitializeClient(const CallbackInfo& info) { if (isStringTooLong(user)) { throw Error::New(info.Env(), "User name is too long"); - return; } if (isStringTooLong(domain)) { throw Error::New(info.Env(), "Domain is too long"); - return; } if (isStringTooLong(password)) { throw Error::New(info.Env(), "Password is too long"); - return; } Value flags_v = options["flags"]; From 20e1421c6f588bd3d34246a5b6b011b7be7f07e6 Mon Sep 17 00:00:00 2001 From: Raymond Zhao <7199958+rzhao271@users.noreply.github.com> Date: Mon, 7 Aug 2023 14:19:17 -0700 Subject: [PATCH 5/5] Add another helper for wstrings --- src/win32/kerberos_win32.cc | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/src/win32/kerberos_win32.cc b/src/win32/kerberos_win32.cc index 9ed902e2..aa653d43 100644 --- a/src/win32/kerberos_win32.cc +++ b/src/win32/kerberos_win32.cc @@ -82,6 +82,10 @@ static bool isStringTooLong(const std::string& str) { return str.length() >= ULONG_MAX; } +static bool isWStringTooLong(const std::wstring& str) { + return str.length() >= ULONG_MAX; +} + void KerberosClient::WrapData(const CallbackInfo& info) { auto state = this->state(); std::string challenge = info[0].ToString(); @@ -128,13 +132,13 @@ void InitializeClient(const CallbackInfo& info) { std::wstring domain = ToWStringWithNonStringAsEmpty(options["domain"]); std::wstring password = ToWStringWithNonStringAsEmpty(options["password"]); - if (isStringTooLong(user)) { + if (isWStringTooLong(user)) { throw Error::New(info.Env(), "User name is too long"); } - if (isStringTooLong(domain)) { + if (isWStringTooLong(domain)) { throw Error::New(info.Env(), "Domain is too long"); } - if (isStringTooLong(password)) { + if (isWStringTooLong(password)) { throw Error::New(info.Env(), "Password is too long"); }