feat(NODE-5505): add compiler warnings and cast lengths

[rzhao271]

Description

Adds some compiler warnings to binding.gyp to resolve downstream BinSkim issues.

What is changing?

This PR adds some additional options to VCCLCompilerTool in binding.gyp, and fixes some type-casting compilation warnings that showed up as a result.

Is there new documentation needed for these changes?

No.

What is the motivation for this change?

VS Code got some BinSkim issues having to do with this library after starting to use it. The issues are:

• Compiler warnings 4244 and 4267 aren't enabled (this PR handles that, and also adds a config flag to always use SHA-256 for file checksums)
• Compiler control guard (CFG) isn't enabled (not sure if relevant to this project, so I didn't add the flags for now). If this issue is relevant to this project, I can create a tracking bug for it.
• Spectre mitigation isn't enabled (waiting on some upstream PRs to be merged first, since this requires an unreleased node-gyp flag to fix). If this issue is relevant to this project, I can create a tracking bug for it.

Double check the following

• Ran npm run check:lint script
• Self-review completed using the steps outlined here
• PR title follows the correct format: type(NODE-xxxx)[!]: description
  • Example: feat(NODE-1234)!: rewriting everything in coffeescript
• Changes are covered by tests
• New TODOs have a related JIRA ticket

[durran]

Thanks @rzhao271 for submitting. I've created a JIRA ticket (NODE-5505) to track this. I think we've decided we want to be super safe and add an assertion in JS that the string lengths cannot exceed the newly cast ULONG sizes. As for your questions, CFG I don't think is relevant here (unless you feel otherwise) but would be great to have a separate ticket filed for Spectre mitigation - I do think that could possibly be relevant.

[rzhao271]

It turns out ULONG_MAX = 2^32 - 1, but the max string length that can be stored in V8 is currently under 2^29, ref https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/String/length.

I also notice that the JS side within this repository doesn't seem to use all string fields with length casts. For example, kerberos.initializeClient(service, initOptions, ...) is never called with a domain option within this repository, but I assume a user could call into the kerberos.initializeClient() function from elsewhere and pass in a domain option.

Therefore, should string length checks be done on the C++ side instead?

[addaleax]

It turns out ULONG_MAX = 2^32 - 1, but the max string length that can be stored in V8 is currently under 2^29, ref https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/String/length.

Yes – one option would be to add something that asserts that buffer.kStringMaxLength is less than 2^32.

Therefore, should string length checks be done on the C++ side instead?

If you want to add checks here, I think that would be more idiomatic, it just needs to happen in a bunch of places I guess.

[durran]

@rzhao271 Looks good, thanks, with just some errors now on Windows:

[2023/08/07 14:53:16.337] c:\data\mci\292dae5a3c99d265424b7f58e8098eb2\src\src\win32\kerberos_win32.cc(131): error C2664: 'bool node_kerberos::isStringTooLong(const std::string &)': cannot convert argument 1 from 'std::wstring' to 'const std::string &' [C:\data\mci\292dae5a3c99d265424b7f58e8098eb2\src\build\kerberos.vcxproj]
[2023/08/07 14:53:16.337]   c:\data\mci\292dae5a3c99d265424b7f58e8098eb2\src\src\win32\kerberos_win32.cc(131): note: Reason: cannot convert from 'std::wstring' to 'const std::string'
[2023/08/07 14:53:16.337]   c:\data\mci\292dae5a3c99d265424b7f58e8098eb2\src\src\win32\kerberos_win32.cc(131): note: No user-defined-conversion operator available that can perform this conversion, or the operator cannot be called
[2023/08/07 14:53:16.337] c:\data\mci\292dae5a3c99d265424b7f58e8098eb2\src\src\win32\kerberos_win32.cc(134): error C2664: 'bool node_kerberos::isStringTooLong(const std::string &)': cannot convert argument 1 from 'std::wstring' to 'const std::string &' [C:\data\mci\292dae5a3c99d265424b7f58e8098eb2\src\build\kerberos.vcxproj]
[2023/08/07 14:53:16.337]   c:\data\mci\292dae5a3c99d265424b7f58e8098eb2\src\src\win32\kerberos_win32.cc(134): note: Reason: cannot convert from 'std::wstring' to 'const std::string'
[2023/08/07 14:53:16.337]   c:\data\mci\292dae5a3c99d265424b7f58e8098eb2\src\src\win32\kerberos_win32.cc(134): note: No user-defined-conversion operator available that can perform this conversion, or the operator cannot be called
[2023/08/07 14:53:16.337] c:\data\mci\292dae5a3c99d265424b7f58e8098eb2\src\src\win32\kerberos_win32.cc(137): error C2664: 'bool node_kerberos::isStringTooLong(const std::string &)': cannot convert argument 1 from 'std::wstring' to 'const std::string &' [C:\data\mci\292dae5a3c99d265424b7f58e8098eb2\src\build\kerberos.vcxproj]
[2023/08/07 14:53:16.337]   c:\data\mci\292dae5a3c99d265424b7f58e8098eb2\src\src\win32\kerberos_win32.cc(137): note: Reason: cannot convert from 'std::wstring' to 'const std::string'
[2023/08/07 14:53:16.337]   c:\data\mci\292dae5a3c99d265424b7f58e8098eb2\src\src\win32\kerberos_win32.cc(137): note: No user-defined-conversion operator available that can perform this conversion, or the operator cannot be called

[durran]

@rzhao271 thanks for the contribution and expedient responses. Much appreciated.