fwknop
fwknop copied to clipboard
fwknop uses unsafe _snprintf() on Windows
In common/common.h
, line 93:
87 /* Some hoops for accommodating Windows
88 */
89 #ifdef WIN32
90 #include <io.h>
91 #define strcasecmp _stricmp
92 #define strncasecmp _strnicmp
93 #define snprintf _snprintf
and in lib/fko_common.h
, line 66:
62 #ifdef WIN32
63 #include <io.h>
64 #define strcasecmp _stricmp
65 #define strncasecmp _strnicmp
66 #define snprintf _snprintf
Windows builds of fwknop are set to use _snprintf()
when snprintf()
is normally used. This is discouraged, and even dangerous, since the implementation of _snprintf()
on Windows is not behaving as expected: it does not automatically terminate strings upon overflows.
See https://msdn.microsoft.com/en-us/library/2ts7cx93.aspx for a complete explanation.
Perhaps unbeknownst to this fact, as it happens many snprintf()
calls in the code are fortunately performed this way:
char buf[BUF_SIZE] = {0}; //initialization with all-zeros apparently
snprintf(buf, sizeof(buf) - 1, "%s", input_string);
On UNIX this wastes the space for one character, since snprintf()
will truncate the buffer at sizeof(buf) - 2
as expected. On Windows luckily even though the buffer was not truncated there, the buffer initialization will apparently guarantee the presence of a terminating character.
Now that Windows supports a C99-conformant implementation of snprintf()
, it should be possible to remove this crude hack. In the meantime every snprintf()
call should probably be reviewed for correctness there.