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

storage/connect/array.cpp

@@ -89,7 +89,7 @@ PARRAY MakeValueArray(PGLOBAL g, PPARM pp)
   /*********************************************************************/
   for (n = 0, parmp = pp; parmp; n++, parmp = parmp->Next)
     if (parmp->Type != valtyp) {
-      sprintf(g->Message, MSG(BAD_PARAM_TYPE), "MakeValueArray", parmp->Type);
+      snprintf(g->Message, sizeof(g->Message), MSG(BAD_PARAM_TYPE), "MakeValueArray", parmp->Type);
       return NULL;
     } else if (valtyp == TYPE_STRING)
       len = MY_MAX(len, strlen((char*)parmp->Value));
@@ -176,7 +176,7 @@ ARRAY::ARRAY(PGLOBAL g, int type, int size, int length, int prec)
       break;
 #endif // 0
     default:  // This is illegal an causes an ill formed array building
-      sprintf(g->Message, MSG(BAD_ARRAY_TYPE), type);
+      snprintf(g->Message, sizeof(g->Message), MSG(BAD_ARRAY_TYPE), type);
       Type = TYPE_ERROR;
       return;
     } // endswitch type
@@ -224,7 +224,7 @@ ARRAY::ARRAY(PGLOBAL g, PQUERY qryp) : CSORT(false)
 //    Value = qryp->GetColValue(0);
 //    break;
     default:  // This is illegal an causes an ill formed array building
-      sprintf(g->Message, MSG(BAD_ARRAY_TYPE), Type);
+      snprintf(g->Message, sizeof(g->Message), MSG(BAD_ARRAY_TYPE), Type);
       Type = TYPE_ERROR;
     } // endswitch type
 
@@ -285,7 +285,7 @@ void ARRAY::Empty(void)
 bool ARRAY::AddValue(PGLOBAL g, PSZ strp)
 {
   if (Type != TYPE_STRING) {
-    sprintf(g->Message, MSG(ADD_BAD_TYPE), GetTypeName(Type), "CHAR");
+    snprintf(g->Message, sizeof(g->Message), MSG(ADD_BAD_TYPE), GetTypeName(Type), "CHAR");
     return true;
   } // endif Type
 
@@ -300,7 +300,7 @@ bool ARRAY::AddValue(PGLOBAL g, PSZ strp)
 bool ARRAY::AddValue(PGLOBAL g, void *p)
 {
   if (Type != TYPE_PCHAR) {
-    sprintf(g->Message, MSG(ADD_BAD_TYPE), GetTypeName(Type), "PCHAR");
+    snprintf(g->Message, sizeof(g->Message), MSG(ADD_BAD_TYPE), GetTypeName(Type), "PCHAR");
     return true;
   } // endif Type
 
@@ -315,7 +315,7 @@ bool ARRAY::AddValue(PGLOBAL g, void *p)
 bool ARRAY::AddValue(PGLOBAL g, short n)
 {
   if (Type != TYPE_SHORT) {
-    sprintf(g->Message, MSG(ADD_BAD_TYPE), GetTypeName(Type), "SHORT");
+    snprintf(g->Message, sizeof(g->Message), MSG(ADD_BAD_TYPE), GetTypeName(Type), "SHORT");
     return true;
   } // endif Type
 
@@ -330,7 +330,7 @@ bool ARRAY::AddValue(PGLOBAL g, short n)
 bool ARRAY::AddValue(PGLOBAL g, int n)
 {
   if (Type != TYPE_INT) {
-    sprintf(g->Message, MSG(ADD_BAD_TYPE), GetTypeName(Type), "INTEGER");
+    snprintf(g->Message, sizeof(g->Message), MSG(ADD_BAD_TYPE), GetTypeName(Type), "INTEGER");
     return true;
   } // endif Type
 
@@ -345,7 +345,7 @@ bool ARRAY::AddValue(PGLOBAL g, int n)
 bool ARRAY::AddValue(PGLOBAL g, double d)
 {
   if (Type != TYPE_DOUBLE) {
-    sprintf(g->Message, MSG(ADD_BAD_TYPE), GetTypeName(Type), "DOUBLE");
+    snprintf(g->Message, sizeof(g->Message), MSG(ADD_BAD_TYPE), GetTypeName(Type), "DOUBLE");
     return true;
   } // endif Type
 
@@ -361,7 +361,7 @@ bool ARRAY::AddValue(PGLOBAL g, double d)
 bool ARRAY::AddValue(PGLOBAL g, PXOB xp)
 {
 	if (Type != xp->GetResultType()) {
-		sprintf(g->Message, MSG(ADD_BAD_TYPE),
+		snprintf(g->Message, sizeof(g->Message), MSG(ADD_BAD_TYPE),
 			GetTypeName(xp->GetResultType()), GetTypeName(Type));
 		return true;
 	} // endif Type
@@ -377,7 +377,7 @@ bool ARRAY::AddValue(PGLOBAL g, PXOB xp)
 bool ARRAY::AddValue(PGLOBAL g, PVAL vp)
 {
   if (Type != vp->GetType()) {
-    sprintf(g->Message, MSG(ADD_BAD_TYPE),
+    snprintf(g->Message, sizeof(g->Message), MSG(ADD_BAD_TYPE),
             GetTypeName(vp->GetType()), GetTypeName(Type));
     return true;
   } // endif Type
@@ -404,7 +404,7 @@ bool ARRAY::GetSubValue(PGLOBAL g, PVAL valp, int *kp)
   PVBLK vblp;
 
   if (Type != TYPE_LIST) {
-    sprintf(g->Message, MSG(NO_SUB_VAL), Type);
+    snprintf(g->Message, sizeof(g->Message), MSG(NO_SUB_VAL), Type);
     return true;
     } // endif Type
 
@@ -500,7 +500,7 @@ bool ARRAY::FilTest(PGLOBAL g, PVAL valp, OPVAL opc, int opm)
       vp = valp;
 
   } else if (opc != OP_EXIST) {
-		sprintf(g->Message, MSG(MISSING_ARG), opc);
+		snprintf(g->Message, sizeof(g->Message), MSG(MISSING_ARG), opc);
 		throw (int)TYPE_ARRAY;
   } else    // OP_EXIST
     return Nval > 0;
@@ -581,7 +581,7 @@ int ARRAY::Convert(PGLOBAL g, int k, PVAL vp)
       Len = 1;
       break;
     default:
-      sprintf(g->Message, MSG(BAD_CONV_TYPE), Type);
+      snprintf(g->Message, sizeof(g->Message), MSG(BAD_CONV_TYPE), Type);
       return TYPE_ERROR;
     } // endswitch k
 

storage/connect/blkfil.cpp

@@ -594,7 +594,7 @@ BLKFILIN::BLKFILIN(PGLOBAL g, PTDBDOS tdbp, int op, int opm, PXOB *xp)
   Type = Arap->GetResultType();
 
   if (Colp->GetResultType() != Type) {
-    sprintf(g->Message, "BLKFILIN: %s", MSG(VALTYPE_NOMATCH));
+    snprintf(g->Message, sizeof(g->Message), "BLKFILIN: %s", MSG(VALTYPE_NOMATCH));
 		throw g->Message;
 	} else if (Colp->GetValue()->IsCi())
     Arap->SetPrecision(g, 1);        // Case insensitive

storage/connect/bson.cpp

@@ -135,7 +135,7 @@ PBVAL BDOC::ParseJson(PGLOBAL g, char* js, size_t lng)
           break;
         } // endif pretty
 
-        sprintf(g->Message, "Unexpected ',' (pretty=%d)", pretty);
+        snprintf(g->Message, sizeof(g->Message), "Unexpected ',' (pretty=%d)", pretty);
         throw 3;
       case '(':
         b = true;
@@ -159,7 +159,7 @@ PBVAL BDOC::ParseJson(PGLOBAL g, char* js, size_t lng)
       }; // endswitch s[i]
 
     if (bvp->Type == TYPE_UNKNOWN)
-      sprintf(g->Message, "Invalid Json string '%.*s'", MY_MIN((int)len, 50), s);
+      snprintf(g->Message, sizeof(g->Message), "Invalid Json string '%.*s'", MY_MIN((int)len, 50), s);
     else if (pretty == 3) {
       for (i = 0; i < 3; i++)
         if (pty[i]) {
@@ -606,7 +606,7 @@ PSZ BDOC::Serialize(PGLOBAL g, PBVAL bvp, char* fn, int pretty)
       b = pretty == 1;
     } else {
       if (!(fs = fopen(fn, "wb"))) {
-        sprintf(g->Message, MSG(OPEN_MODE_ERROR),
+        snprintf(g->Message, sizeof(g->Message), MSG(OPEN_MODE_ERROR),
           "w", (int)errno, fn);
         strcat(strcat(g->Message, ": "), strerror(errno));
         throw 2;

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);