Safe String Functions (+ Modificated version) #15
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Added demo code from Safe String Functions section
There is in "Chapter 1 - Safe String Functions" section is located code example which demonstrates the reliability of security
level of newer string functions in comparison with other classic string functions:
void wmain(int argc, const wchar_t* argv[])
{
// assume arc >= 2 for this demo
WCHAR buffer[32];
wcscpy_s(buffer, argv[1]); // C++ version aware of static buffers
WCHAR* buffer2 = (WCHAR*)malloc(32 * sizeof(WCHAR));
// wcscpy_s(buffer, argv[1]); // does not compile
wcscpy_s(buffer2, 32, argv[1]); // size in characters (not bytes)
free(buffer2);
}
But I've noticed that in dynamic memory allocation part arises the "null pointer dereferencing" which can cause to undefined
behaviour later. The first appeal to the memory by null pointer or in a differ terms dereferencing the null pointer usually
causes the "structured exception" in Windows systems. In "Microsoft Visual Studio Community 2019 Version 16.11.29" environment
compiler generates "Warning C6387" coherented with "NULL value of buffer2", since buffer2 is the argument of wcscpy_s and due to
SAL for wcscpy_s function the buffer2 can not be NULL related with "Warning C6011" where it took its initial point which corresponds
to "null pointer dereferencing":
// corecrt_wstring.h
_Check_return_wat_
_ACRTIMP errno_t __cdecl wcscpy_s(
_Out_writes_z_(_SizeInWords) wchar_t* _Destination,
_In_ rsize_t _SizeInWords,
_In_z_ wchar_t const* _Source
);
// sal.h
#define _Out_writes_z_(size) _SAL2_Source_(_Out_writes_z_, (size), _Pre_cap_(size) _Post_valid_impl_ _Post_z_)
This part is just about informational purposes for the other readers:
0x00000000 - 0x00000FFF in x86_32 and 0x0000000000000000 - 0x000000000000FFFF in x86_64: In Windows 9x architecture this region
of memory for user level was not available for accessing from user mode because of though it's part of user mode region, but
it's reserved by OS and played a trap role to detect a null pointers, plus to it contains the null address. Although they are have
the close meaning in Windows NT architecture this region is for detection the pointers with wrong values. Trying to access this
memory section causes to raising an "memory access violation" exeption. In this case null pointer must be store the value which
indicates the fault and serves that this pointer is invalid. That is the target of reservation the null address by the system and
of course this is the null pointer related section. But this region is not only included null address. Following the null address,
are located the 65535 addresses. That is why the size of this region is 64KiB. It happens due to "data alignment" within comply
the sophisticated rules to treatment all addresses with the same way. Despite to it's non-optimaze, to deny access to some of
addresses all 65536 addresses should to be denied, but this is small amount of memory in compairing with large total amount of
user level memory. Null pointer dereferencing may causes a lot of problems started from large or small amount of memory corruptions
associated even with compiler optimizations which according to the compiler the nullptr can not be dereferenced and could apply
swapping of assignments which causes the "structural exception", but it will happen too late and despite this fact the datas in
memory will be corrupted. That is why applying SEH (-> VEH) is not a single option at some in this case. Kindred situations related
with compiler optimizations applying swap in assignments could be arises with the use of memset function also.
I suggest make this kind of changes due to on my opinion this code snippet will be more secure hence clear in versions like
below.
// 1st modificated version just for sample, but actually working code sample and more secure too.
#include "pch.h"
void wmain(int argc, const wchar_t* argv[])
{
WCHAR buffer[32] = {'\0'};
wcscpy_s(buffer, argv[1]);
WCHAR* buffer2 = (WCHAR*)malloc(32 * sizeof(WCHAR));
if (buffer2){
memset(buffer2, 0, sizeof(WCHAR));
wcscpy_s(buffer2, 32, argv[1]);
free(buffer2);
}
}
// 2nd modificated version
#include "pch.h"
void wmain(int argc, const wchar_t* argv[])
{
WCHAR buffer[32] = {'\0'};
wcscpy_s(buffer, argv[1]);
WCHAR* buffer2 = (WCHAR*)calloc(32, sizeof(WCHAR));
if (buffer2){
wcscpy_s(buffer2, 32, argv[1]);
free(buffer2);
}
}
Added demo code from RAII for Handles section
Modification kindred with addings (all tested in Visual Studio 2019): 1) "noexcept" operator to the necessary operations, such as destructor, move constructor and move assignment operator. Due to there was generates warning C26439: https://learn.microsoft.com/en-us/cpp/code-quality/c26439?view=msvc-170#:~:text=Special%20kinds%20of%20operations Importance of adding noexcept operations: https://en.cppreference.com/w/cpp/language/operators#:~:text=%26%26%20other)-,noexcept,-%7B%0A%20%20%20%20//%20Guard%20self https://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#Rf-noexcept:~:text=low%2Dlevel%20functions.-,Note,-Destructors%2C%20swap https://en.cppreference.com/w/cpp/language/noexcept#:~:text=The%20noexcept%20operator%20performs%20a,some%20types%20but%20not%20others. 2) The main function have to return type an "int" in general, because of that is why there was a compile error C2561 which solved via changing the "return;" line to "return 1;" for int main() function: https://learn.microsoft.com/en-us/cpp/error-messages/compiler-errors-2/compiler-error-c2561?view=msvc-170&f1url=%3FappId%3DDev16IDEF1%26l%3DRU-RU%26k%3Dk(C2561)%26rd%3Dtrue https://learn.microsoft.com/en-us/cpp/error-messages/compiler-warnings/compiler-warning-level-1-c4326?view=msvc-170&f1url=%3FappId%3DDev16IDEF1%26l%3DRU-RU%26k%3Dk(C4326)%26rd%3Dtrue 3) The "Get()" method was replaced with "operator()" overloading related with adding implicit conversion to HANDLE as you mentioned: HANDLE operator()() const { return _h; }
1) Added "if statement" by reason of Warning C28183 - https://learn.microsoft.com/en-us/cpp/code-quality/c28183?view=msvc 170&f1url=%3FappId%3DDev16IDEF1%26l%3DRU-RU%26k%3Dk(C28183)%26rd%3Dtrue 2) Added 2nd condition to the statement due to Warning C6387 - https://learn.microsoft.com/en-us/cpp/code-quality/c6387?view=msvc-170&f1url=%3FappId%3DDev16IDEF1%26l%3DRU-RU%26k%3Dk(C6387)%26rd%3Dtrue 3) There are would be occurred compiler warnings as "Warning C6262" if the stack frame size limit is appointed for "user mode" is exceeded - 16 KB = 16000 Bytes, 16 KiB = 16384 Bytes, but there are 16420 Bytes in main function. A large stack frame can cause a stack overflow. This can be solved by transferring some data (a group of data) located on the "stack" to one of the dynamic segments in either the "heap" or the "global space" or by encreased the stack size "/analyze:stacksize 32768". When "(w)char" array is "initialized" in C++, '\0' is always appended to the end, either by "manual" or "compiler". Therefore, there must be a 1 byte reservation for '\0' at the end of the array. This should be taken into when the array is "initialized". 4) Changed the "WCHAR text[8192] = { 0 };" to "WCHAR text[8192] = { '\0' };" through the definitions in related headers such as winnt.h, wtypes.h, WTypesBase.h. For example one of them: winnnt.h: // // UNICODE (Wide Character) types // #ifndef _MAC typedef wchar_t WCHAR; // wc, 16-bit UNICODE character
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Added demo code sample from "Chapter 1 - Safe String Functions" section.
Added modificated version of the same demo code snippet.