Use memory safe snprintf() in Connect Engine

This commit replaces sprintf(buf, ...) with
snprintf(buf, sizeof(buf), ...),
specifically in the "easy" cases where buf is allocated with a size
known at compile time.

The changes make sure we are not write outside array/string bounds which
will lead to undefined behaviour. In case the code is trying to write
outside bounds - safe version of functions simply cut the string
messages so we process this gracefully.

All new code of the whole pull request, including one or several files
that are either new files or modified ones, are contributed under the BSD-new
license.  I am contributing on behalf of my employer Amazon Web Services,
Inc.

bsonudf.cpp warnings cleanup by Daniel Black

Reviewer: Daniel Black

1 files changed, 17 insertions, 19 deletions

diff --git a/storage/connect/bsonudf.cpp b/storage/connect/bsonudf.cpp
index 491f388bc5f..7d930d1f410 100644
--- a/storage/connect/bsonudf.cpp
+++ b/storage/connect/bsonudf.cpp
@@ -238,7 +238,7 @@ my_bool BJNX::SetArrayOptions(PGLOBAL g, char* p, int i, PSZ nm)
 		jnp->Rank = atoi(p) - B;
 		jnp->Op = OP_EQ;
 	} else if (Wr) {
-		sprintf(g->Message, "Invalid specification %s in a write path", p);
+		snprintf(g->Message, sizeof(g->Message), "Invalid specification %s in a write path", p);
 		return true;
 	} else if (n == 1) {
 		// Set the Op value;
@@ -251,7 +251,7 @@ my_bool BJNX::SetArrayOptions(PGLOBAL g, char* p, int i, PSZ nm)
 		case '#': jnp->Op = OP_NUM;  break;
 		case '*': jnp->Op = OP_EXP;  break;
 		default:
-			sprintf(g->Message, "Invalid function specification %c", *p);
+			snprintf(g->Message, sizeof(g->Message), "Invalid function specification %c", *p);
 			return true;
 		} // endswitch *p
 
@@ -287,7 +287,7 @@ my_bool BJNX::ParseJpath(PGLOBAL g)
 {
 	char* p, * p1 = NULL, * p2 = NULL, * pbuf = NULL;
 	int     i;
-	my_bool a, mul = false;
+	my_bool a;
 
 	if (Parsed)
 		return false;                       // Already done
@@ -343,7 +343,7 @@ my_bool BJNX::ParseJpath(PGLOBAL g)
 
 		} else if (*p == '*') {
 			if (Wr) {
-				sprintf(g->Message, "Invalid specification %c in a write path", *p);
+				snprintf(g->Message, sizeof(g->Message), "Invalid specification %c in a write path", *p);
 				return true;
 			} else     			// Return JSON
 				Nodes[i].Op = OP_XX;
@@ -541,7 +541,6 @@ PVAL BJNX::GetColumnValue(PGLOBAL g, PBVAL row, int i)
 /*********************************************************************************/
 PBVAL BJNX::GetRowValue(PGLOBAL g, PBVAL row, int i)
 {
-	my_bool expd = false;
 	PBVAL   bap;
 	PBVAL   vlp = NULL;
 
@@ -596,7 +595,7 @@ PBVAL BJNX::GetRowValue(PGLOBAL g, PBVAL row, int i)
 			vlp = row;
 			break;
 		default:
-			sprintf(g->Message, "Invalid row JSON type %d", row->Type);
+			snprintf(g->Message, sizeof(g->Message), "Invalid row JSON type %d", row->Type);
 			vlp = NULL;
 		} // endswitch Type
 
@@ -841,7 +840,7 @@ PBVAL BJNX::GetRow(PGLOBAL g)
 			val = MVP(row->To_Val);
 			break;
 		default:
-			sprintf(g->Message, "Invalid row JSON type %d", row->Type);
+			snprintf(g->Message, sizeof(g->Message), "Invalid row JSON type %d", row->Type);
 			val = NULL;
 		} // endswitch Type
 
@@ -1033,7 +1032,7 @@ my_bool BJNX::CheckPath(PGLOBAL g)
 				val = row;
 				break;
 			default:
-				sprintf(g->Message, "Invalid row JSON type %d", row->Type);
+				snprintf(g->Message, sizeof(g->Message), "Invalid row JSON type %d", row->Type);
 		} // endswitch Type
 
 		if (i < Nod-1)
@@ -1062,7 +1061,7 @@ my_bool BJNX::CheckPath(PGLOBAL g, UDF_ARGS *args, PBVAL jsp, PBVAL& jvp, int n)
 					return true;
 
 				if (!(jvp = GetJson(g))) {
-					sprintf(g->Message, "No sub-item at '%s'", path);
+					snprintf(g->Message, sizeof(g->Message), "No sub-item at '%s'", path);
 					return true;
 				} else
 					return false;
@@ -1084,7 +1083,7 @@ my_bool BJNX::CheckPath(PGLOBAL g, UDF_ARGS *args, PBVAL jsp, PBVAL& jvp, int n)
 PSZ BJNX::Locate(PGLOBAL g, PBVAL jsp, PBVAL jvp, int k)
 {
 	PSZ     str = NULL;
-	my_bool b = false, err = true;
+	my_bool err = true;
 
 	g->Message[0] = 0;
 
@@ -1205,7 +1204,7 @@ my_bool BJNX::LocateValue(PGLOBAL g, PBVAL jvp)
 PSZ BJNX::LocateAll(PGLOBAL g, PBVAL jsp, PBVAL bvp, int mx)
 {
 	PSZ     str = NULL;
-	my_bool b = false, err = true;
+	my_bool err = true;
 	PJPN    jnp;
 
 	if (!jsp) {
@@ -1630,7 +1629,7 @@ PBVAL BJNX::ParseJsonFile(PGLOBAL g, char *fn, int& pty, size_t& len)
 		DWORD rc = GetLastError();
 
 		if (!(*g->Message))
-			sprintf(g->Message, MSG(OPEN_MODE_ERROR), "map", (int)rc, fn);
+			snprintf(g->Message, sizeof(g->Message), MSG(OPEN_MODE_ERROR), "map", (int)rc, fn);
 
 		return NULL;
 	} // endif hFile
@@ -1650,7 +1649,7 @@ PBVAL BJNX::ParseJsonFile(PGLOBAL g, char *fn, int& pty, size_t& len)
 
 	if (!memory) {
 		CloseFileHandle(hFile);
-		sprintf(g->Message, MSG(MAP_VIEW_ERROR), fn, GetLastError());
+		snprintf(g->Message, sizeof(g->Message), MSG(MAP_VIEW_ERROR), fn, GetLastError());
 		return NULL;
 	} // endif Memory
 
@@ -3045,7 +3044,7 @@ my_bool bson_test_init(UDF_INIT* initid, UDF_ARGS* args, char* message) {
 
 char* bson_test(UDF_INIT* initid, UDF_ARGS* args, char* result,
 	unsigned long* res_length, char* is_null, char* error) {
-	char* str = NULL, * sap = NULL, * fn = NULL;
+	char* str = NULL, *fn = NULL;
 	int     pretty = 1;
 	PBVAL   bvp;
 	PGLOBAL g = (PGLOBAL)initid->ptr;
@@ -4765,7 +4764,7 @@ char *bfile_bjson(UDF_INIT *initid, UDF_ARGS *args, char *result,
 
 					if (!fgets(buf, lrecl, fin)) {
 						if (!feof(fin)) {
-							sprintf(g->Message, "Error %d reading %zd bytes from %s",
+							snprintf(g->Message, sizeof(g->Message), "Error %d reading %zu bytes from %s",
 								errno, lrecl, fn);
 							str = strcpy(result, g->Message);
 						}	else
@@ -4777,11 +4776,11 @@ char *bfile_bjson(UDF_INIT *initid, UDF_ARGS *args, char *result,
 							binszp = newloc - (size_t)jsp;
 
 							if (fwrite(&binszp, sizeof(binszp), 1, fout) != 1) {
-								sprintf(g->Message, "Error %d writing %zd bytes to %s", 
+								snprintf(g->Message, sizeof(g->Message), "Error %d writing %zu bytes to %s",
 									errno, sizeof(binszp), ofn);
 								str = strcpy(result, g->Message);
 							} else if (fwrite(jsp, binszp, 1, fout) != 1) {
-								sprintf(g->Message, "Error %d writing %zd bytes to %s", 
+								snprintf(g->Message, sizeof(g->Message), "Error %d writing %zu bytes to %s",
 									errno, binszp, ofn);
 								str = strcpy(result, g->Message);
 							} else
@@ -5043,7 +5042,7 @@ char* bbin_array_add_values(UDF_INIT* initid, UDF_ARGS* args, char* result,
 		if (!CheckMemory(g, initid, args, args->arg_count, true)) {
 			uint  i = 0;
 			BJNX  bnx(g);
-			PBVAL arp, top, jvp = NULL;
+			PBVAL arp, top;
 			PBVAL bvp = bnx.MakeValue(args, 0, true, &top);
 
 			if (bvp->Type == TYPE_JAR) {
@@ -5667,7 +5666,6 @@ char *bbin_get_item(UDF_INIT *initid, UDF_ARGS *args, char *result,
 	if (g->Xchk) {
 		bsp = (PBSON)g->Xchk;
 	} else if (!CheckMemory(g, initid, args, 1, true, true)) {
-		char *path = MakePSZ(g, args, 1);
 		BJNX  bnx(g, NULL, TYPE_STRING, initid->max_length);
 		PBVAL top, jvp = NULL;
 		PBVAL jsp = bnx.MakeValue(args, 0, true, &top);