fix buffer underrun for wrong received buffer size (#199)

This commit fixes an underrun in the utility function copying a
wide-char string to the client application.

The impact is limited and it can only happen if the
received size of the client buffer is of the wrong size (<=0).

The commit also fixes a logging error happening if this function needs
to truncate the output. The macro for a format specifier with no
precision was provided instead of the needed one with precision. This
could potentially crash the client application in case the string to be
logged in not 0-terminated, logging is explicitely
enabled at an INFO (or higher) level and truncation needs to be applied

Author: bpintea

driver/util.c

@@ -524,7 +524,7 @@ size_t json_escape_overlapping(char *str, size_t inlen, size_t outlen)
  * wstr according to avaialble space and indicates the available bytes to copy
  * back into provided buffer (if not NULL).
  */
-SQLRETURN write_wstr(SQLHANDLE hnd, SQLWCHAR *dest, wstr_st *src,
+SQLRETURN TEST_API write_wstr(SQLHANDLE hnd, SQLWCHAR *dest, wstr_st *src,
 	SQLSMALLINT /*B*/avail, SQLSMALLINT /*B*/*usedp)
 {
 	size_t wide_avail;
@@ -533,8 +533,8 @@ SQLRETURN write_wstr(SQLHANDLE hnd, SQLWCHAR *dest, wstr_st *src,
 		"len; out-len @0x%p.", src->cnt, LWSTR(src), dest, avail, usedp);
 
 	/* cnt must not count the 0-term (XXX: ever need to copy 0s?) */
-	assert(src->cnt <= 0 || src->str[src->cnt - 1]);
-	assert(src->cnt <= 0 || src->str[src->cnt] == 0);
+	assert(src->cnt <= 0 || src->str[src->cnt - 1] != L'\0');
+	assert(src->cnt <= 0 || src->str[src->cnt] == L'\0');
 
 	if (usedp) {
 		/* how many bytes are available to return (not how many would be
@@ -547,20 +547,22 @@ SQLRETURN write_wstr(SQLHANDLE hnd, SQLWCHAR *dest, wstr_st *src,
 
 	if (dest) {
 		/* needs to be multiple of SQLWCHAR units (2 on Win) */
-		if (avail % sizeof(SQLWCHAR)) {
-			ERRH(hnd, "invalid buffer length provided: %d.", avail);
+		if (avail < 0 || avail % sizeof(SQLWCHAR)) {
+			ERRH(hnd, "invalid buffer length provided: %hd.", avail);
 			RET_HDIAGS(hnd, SQL_STATE_HY090);
 		} else {
 			wide_avail = avail/sizeof(SQLWCHAR);
 		}
 
 		/* '=' (in <=), since src->cnt doesn't count the \0 */
 		if (wide_avail <= src->cnt) {
-			wcsncpy(dest, src->str, wide_avail - /* 0-term */1);
-			dest[wide_avail - 1] = 0;
+			if (0 < wide_avail) {
+				wcsncpy(dest, src->str, wide_avail - /* 0-term */1);
+				dest[wide_avail - 1] = L'\0';
+			}
 
 			INFOH(hnd, "not enough buffer size to write required string (plus "
-				"terminator): `" LWPD "` [%zu]; available: %zu.",
+				"terminator): `" LWPDL "` [%zu]; available: %zu.",
 				LWSTR(src), src->cnt, wide_avail);
 			RET_HDIAGS(hnd, SQL_STATE_01004);
 		} else {

driver/util.h

@@ -270,7 +270,7 @@ size_t json_escape_overlapping(char *str, size_t inlen, size_t outlen);
  * wstr according to avaialble space and indicates the available bytes to copy
  * back into provided buffer (if not NULL).
  */
-SQLRETURN write_wstr(SQLHANDLE hnd, SQLWCHAR *dest, wstr_st *src,
+SQLRETURN TEST_API write_wstr(SQLHANDLE hnd, SQLWCHAR *dest, wstr_st *src,
 	SQLSMALLINT /*B*/avail, SQLSMALLINT /*B*/*usedp);
 
 /*

test/test_util.cc

@@ -6,6 +6,7 @@
 
 extern "C" {
 #include "util.h"
+#include "handles.h"
 } // extern C
 
 #include <gtest/gtest.h>
@@ -105,6 +106,55 @@ TEST_F(Util, wstr_to_utf8_no_nts) {
 	free(dst.str);
 }
 
+TEST_F(Util, write_wstr_invalid_avail) {
+	wstr_st src = WSTR_INIT(SRC_STR);
+	SQLWCHAR dst[sizeof(SRC_STR)];
+	esodbc_env_st env = {0};
+	SQLSMALLINT used;
+
+	SQLRETURN ret = write_wstr(&env, dst, &src, sizeof(*dst) + 1, &used);
+	ASSERT_FALSE(SQL_SUCCEEDED(ret));
+}
+
+TEST_F(Util, write_wstr_0_avail) {
+	wstr_st src = WSTR_INIT(SRC_STR);
+	SQLWCHAR dst[sizeof(SRC_STR)];
+	esodbc_env_st env = {0};
+	SQLSMALLINT used;
+
+	SQLRETURN ret = write_wstr(&env, dst, &src, /*avail*/0, &used);
+	assert(SQL_SUCCEEDED(ret));
+	ASSERT_EQ(used, src.cnt * sizeof(*dst));
+}
+
+TEST_F(Util, write_wstr_trunc) {
+	wstr_st src = WSTR_INIT(SRC_STR);
+	SQLWCHAR dst[sizeof(SRC_STR)];
+	esodbc_env_st env = {0};
+	SQLSMALLINT used;
+
+	SQLRETURN ret = write_wstr(&env, dst, &src,
+			(SQLSMALLINT)((src.cnt - 1) * sizeof(*dst)), &used);
+	assert(SQL_SUCCEEDED(ret));
+	ASSERT_EQ(used, src.cnt * sizeof(*dst));
+	ASSERT_EQ(dst[src.cnt - 2], L'\0');
+	ASSERT_EQ(wcsncmp(src.str, dst, src.cnt - 2), 0);
+}
+
+TEST_F(Util, write_wstr_copy) {
+	wstr_st src = WSTR_INIT(SRC_STR);
+	SQLWCHAR dst[sizeof(SRC_STR)];
+	esodbc_env_st env = {0};
+	SQLSMALLINT used;
+
+	SQLRETURN ret = write_wstr(&env, dst, &src, (SQLSMALLINT)sizeof(dst),
+			&used);
+	assert(SQL_SUCCEEDED(ret));
+	ASSERT_EQ(used, src.cnt * sizeof(*dst));
+	ASSERT_EQ(dst[src.cnt], L'\0');
+	ASSERT_EQ(wcscmp(src.str, dst), 0);
+}
+
 TEST_F(Util, utf8_to_wstr_unicode) {
 #undef SRC_STR
 #undef SRC_AID
@@ -121,8 +171,6 @@ TEST_F(Util, utf8_to_wstr_unicode) {
 	free(dst_wc.str);
 }
 
-
-
 TEST_F(Util, ascii_c2w2c)
 {
 #undef SRC_STR