Skip to content

Commit 61b8a01

Browse files
committed
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 (cherry picked from commit ea2814b) Merge conflicts: - test/test_util.cc fixed by removing test for utf8_to_wstr(), not present in 6.x.
1 parent 361548c commit 61b8a01

File tree

3 files changed

+61
-9
lines changed

3 files changed

+61
-9
lines changed

driver/util.c

+10-8
Original file line numberDiff line numberDiff line change
@@ -526,14 +526,14 @@ size_t json_escape_overlapping(char *str, size_t inlen, size_t outlen)
526526
* wstr according to avaialble space and indicates the available bytes to copy
527527
* back into provided buffer (if not NULL).
528528
*/
529-
SQLRETURN write_wstr(SQLHANDLE hnd, SQLWCHAR *dest, wstr_st *src,
529+
SQLRETURN TEST_API write_wstr(SQLHANDLE hnd, SQLWCHAR *dest, wstr_st *src,
530530
SQLSMALLINT /*B*/avail, SQLSMALLINT /*B*/*usedp)
531531
{
532532
size_t wide_avail;
533533

534534
/* cnt must not count the 0-term (XXX: ever need to copy 0s?) */
535-
assert(src->cnt <= 0 || src->str[src->cnt - 1]);
536-
assert(src->cnt <= 0 || src->str[src->cnt] == 0);
535+
assert(src->cnt <= 0 || src->str[src->cnt - 1] != L'\0');
536+
assert(src->cnt <= 0 || src->str[src->cnt] == L'\0');
537537

538538
DBGH(hnd, "copying %zd wchars (`" LWPDL "`) into buffer @0x%p, of %dB "
539539
"len; out-len @0x%p.", src->cnt, LWSTR(src), dest, avail, usedp);
@@ -549,20 +549,22 @@ SQLRETURN write_wstr(SQLHANDLE hnd, SQLWCHAR *dest, wstr_st *src,
549549

550550
if (dest) {
551551
/* needs to be multiple of SQLWCHAR units (2 on Win) */
552-
if (avail % sizeof(SQLWCHAR)) {
553-
ERRH(hnd, "invalid buffer length provided: %d.", avail);
552+
if (avail < 0 || avail % sizeof(SQLWCHAR)) {
553+
ERRH(hnd, "invalid buffer length provided: %hd.", avail);
554554
RET_HDIAGS(hnd, SQL_STATE_HY090);
555555
} else {
556556
wide_avail = avail/sizeof(SQLWCHAR);
557557
}
558558

559559
/* '=' (in <=), since src->cnt doesn't count the \0 */
560560
if (wide_avail <= src->cnt) {
561-
wcsncpy(dest, src->str, wide_avail - /* 0-term */1);
562-
dest[wide_avail - 1] = 0;
561+
if (0 < wide_avail) {
562+
wcsncpy(dest, src->str, wide_avail - /* 0-term */1);
563+
dest[wide_avail - 1] = L'\0';
564+
}
563565

564566
INFOH(hnd, "not enough buffer size to write required string (plus "
565-
"terminator): `" LWPD "` [%zu]; available: %zu.",
567+
"terminator): `" LWPDL "` [%zu]; available: %zu.",
566568
LWSTR(src), src->cnt, wide_avail);
567569
RET_HDIAGS(hnd, SQL_STATE_01004);
568570
} else {

driver/util.h

+1-1
Original file line numberDiff line numberDiff line change
@@ -252,7 +252,7 @@ size_t json_escape_overlapping(char *str, size_t inlen, size_t outlen);
252252
* wstr according to avaialble space and indicates the available bytes to copy
253253
* back into provided buffer (if not NULL).
254254
*/
255-
SQLRETURN write_wstr(SQLHANDLE hnd, SQLWCHAR *dest, wstr_st *src,
255+
SQLRETURN TEST_API write_wstr(SQLHANDLE hnd, SQLWCHAR *dest, wstr_st *src,
256256
SQLSMALLINT /*B*/avail, SQLSMALLINT /*B*/*usedp);
257257

258258
/*

test/test_util.cc

+50
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66

77
extern "C" {
88
#include "util.h"
9+
#include "handles.h"
910
} // extern C
1011

1112
#include <gtest/gtest.h>
@@ -105,6 +106,55 @@ TEST_F(Util, wstr_to_utf8_no_nts) {
105106
free(dst.str);
106107
}
107108

109+
TEST_F(Util, write_wstr_invalid_avail) {
110+
wstr_st src = WSTR_INIT(SRC_STR);
111+
SQLWCHAR dst[sizeof(SRC_STR)];
112+
esodbc_env_st env = {0};
113+
SQLSMALLINT used;
114+
115+
SQLRETURN ret = write_wstr(&env, dst, &src, sizeof(*dst) + 1, &used);
116+
ASSERT_FALSE(SQL_SUCCEEDED(ret));
117+
}
118+
119+
TEST_F(Util, write_wstr_0_avail) {
120+
wstr_st src = WSTR_INIT(SRC_STR);
121+
SQLWCHAR dst[sizeof(SRC_STR)];
122+
esodbc_env_st env = {0};
123+
SQLSMALLINT used;
124+
125+
SQLRETURN ret = write_wstr(&env, dst, &src, /*avail*/0, &used);
126+
assert(SQL_SUCCEEDED(ret));
127+
ASSERT_EQ(used, src.cnt * sizeof(*dst));
128+
}
129+
130+
TEST_F(Util, write_wstr_trunc) {
131+
wstr_st src = WSTR_INIT(SRC_STR);
132+
SQLWCHAR dst[sizeof(SRC_STR)];
133+
esodbc_env_st env = {0};
134+
SQLSMALLINT used;
135+
136+
SQLRETURN ret = write_wstr(&env, dst, &src,
137+
(SQLSMALLINT)((src.cnt - 1) * sizeof(*dst)), &used);
138+
assert(SQL_SUCCEEDED(ret));
139+
ASSERT_EQ(used, src.cnt * sizeof(*dst));
140+
ASSERT_EQ(dst[src.cnt - 2], L'\0');
141+
ASSERT_EQ(wcsncmp(src.str, dst, src.cnt - 2), 0);
142+
}
143+
144+
TEST_F(Util, write_wstr_copy) {
145+
wstr_st src = WSTR_INIT(SRC_STR);
146+
SQLWCHAR dst[sizeof(SRC_STR)];
147+
esodbc_env_st env = {0};
148+
SQLSMALLINT used;
149+
150+
SQLRETURN ret = write_wstr(&env, dst, &src, (SQLSMALLINT)sizeof(dst),
151+
&used);
152+
assert(SQL_SUCCEEDED(ret));
153+
ASSERT_EQ(used, src.cnt * sizeof(*dst));
154+
ASSERT_EQ(dst[src.cnt], L'\0');
155+
ASSERT_EQ(wcscmp(src.str, dst), 0);
156+
}
157+
108158
TEST_F(Util, ascii_c2w2c)
109159
{
110160
#undef SRC_STR

0 commit comments

Comments
 (0)