MDEV-27612 Connect : check buffer sizes, fix string format errors

13 files changed, 280 insertions, 17 deletions

diff --git a/storage/connect/bsonudf.cpp b/storage/connect/bsonudf.cpp
index ed795edb363..7ae38bc65de 100644
--- a/storage/connect/bsonudf.cpp
+++ b/storage/connect/bsonudf.cpp
@@ -201,7 +201,7 @@ my_bool BJNX::SetArrayOptions(PGLOBAL g, char* p, int i, PSZ nm)
 			p[--n] = 0;
 		} else if (!IsNum(p)) {
 			// Wrong array specification
-			sprintf(g->Message, "Invalid array specification %s", p);
+			snprintf(g->Message, sizeof(g->Message), "Invalid array specification %s", p);
 			return true;
 		} // endif p
 
diff --git a/storage/connect/ha_connect.cc b/storage/connect/ha_connect.cc
index 5493f3b6730..ac3769c399e 100644
--- a/storage/connect/ha_connect.cc
+++ b/storage/connect/ha_connect.cc
@@ -5707,10 +5707,10 @@ static int connect_assisted_discovery(handlerton *, THD* thd,
 		if (ttp == TAB_UNDEF && !topt->http) {
 			topt->type= (src) ? "MYSQL" : (tab) ? "PROXY" : "DOS";
 			ttp= GetTypeID(topt->type);
-			sprintf(g->Message, "No table_type. Was set to %s", topt->type);
+			snprintf(g->Message, sizeof(g->Message), "No table_type. Was set to %s", topt->type);
 			push_warning(thd, Sql_condition::WARN_LEVEL_NOTE, 0, g->Message);
 		} else if (ttp == TAB_NIY) {
-			sprintf(g->Message, "Unsupported table type %s", topt->type);
+			snprintf(g->Message, sizeof(g->Message), "Unsupported table type %s", topt->type);
 			rc= HA_ERR_INTERNAL_ERROR;
 			goto err;
 #if defined(REST_SUPPORT)
diff --git a/storage/connect/jsonudf.cpp b/storage/connect/jsonudf.cpp
index 82492cab6ef..363a853cfa7 100644
--- a/storage/connect/jsonudf.cpp
+++ b/storage/connect/jsonudf.cpp
@@ -123,7 +123,7 @@ my_bool JSNX::SetArrayOptions(PGLOBAL g, char *p, int i, PSZ nm)
 			p[--n] = 0;
 		} else if (!IsNum(p)) {
 			// Wrong array specification
-			sprintf(g->Message, "Invalid array specification %s", p);
+			snprintf(g->Message, sizeof(g->Message), "Invalid array specification %s", p);
 			return true;
 		} // endif p
 
diff --git a/storage/connect/myconn.cpp b/storage/connect/myconn.cpp
index 945c4e698be..438f0ff2b9a 100644
--- a/storage/connect/myconn.cpp
+++ b/storage/connect/myconn.cpp
@@ -399,15 +399,19 @@ PQRYRES SrcColumns(PGLOBAL g, const char *host, const char *db,
   int     w;
   MYSQLC  myc;
   PQRYRES qrp = NULL;
+  const char *p;
 
   if (!port)
     port = mysqld_port;
 
 	if (!strnicmp(srcdef, "select ", 7) || strstr(srcdef, "%s")) {
-    query = (char *)PlugSubAlloc(g, NULL, strlen(srcdef) + 10);
+		query = (char *)PlugSubAlloc(g, NULL, strlen(srcdef) + 10);
 
-		if (strstr(srcdef, "%s"))
-			sprintf(query, srcdef, "1=1");			 // dummy where clause
+		if ((p= strstr(srcdef, "%s")))
+		{
+			/* Replace %s with 1=1 */
+			sprintf(query, "%.*s1=1%s", (int) (p - srcdef), srcdef, p + 2); // dummy where clause
+		}
 		else 
 		  strcpy(query, srcdef);
 
diff --git a/storage/connect/mysql-test/connect/r/misc.result b/storage/connect/mysql-test/connect/r/misc.result
new file mode 100644
index 00000000000..6b6372f6e41
--- /dev/null
+++ b/storage/connect/mysql-test/connect/r/misc.result
@@ -0,0 +1,54 @@
+execute immediate concat('create table t engine=CONNECT table_type=JSON',REPEAT('1',5000),
+' FILE_NAME=''users.json'' HTTP=''http://localhost:4142'' URI=''/users''');
+ERROR HY000: Unsupported table type JSON1111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111
+execute immediate concat('create table t  engine=CONNECT table_type=OEM module=''libname''
+Option_list=''Myopt=foo'' subtype=''MYTYPE',REPEAT('1', 10000), '''');
+ERROR HY000: Subtype string too long
+execute immediate concat('create table t engine=CONNECT table_type=DBF file_name=''',
+REPLACE(@@secure_file_priv,'\\','/'),'cust.dbf', REPEAT('1', 10000), '''');
+ERROR HY000: Cannot open 
+create table t engine=connect table_type=mysql
+CONNECTION='mysql://root@localhost:MASTER_MYPORT/test/foobar'
+  SRCDEF='SELECT 1,''%n'' FROM DUAL WHERE %s';
+select *from t;
+ERROR HY000: Got error 174 'MakeSQL: Wrong place holders specification' from CONNECT
+drop table t;
+create table t engine=connect table_type=mysql
+CONNECTION='mysql://root@localhost:MASTER_MYPORT/test/foobar'
+  SRCDEF='SELECT 1,%n FROM DUAL WHERE %s';
+ERROR HY000: (1064) You have an error in your SQL syntax; check the manual that corresponds to your MariaDB server version for the right syntax to use near '%n FROM DUAL WHERE 1=1 LIMIT 0' at line 1 [SELECT 1,%n FROM DUAL WHERE 1=1 LIMIT 0]
+create table t engine=connect table_type=mysql
+CONNECTION='mysql://root@localhost:MASTER_MYPORT/test/foobar'
+  SRCDEF='SELECT 1 FROM DUAL WHERE %s';
+select *from t;
+1
+1
+drop table t;
+create table beers (
+`Name` char(16) xpath='brandName',
+`Origin` char(16) xpath='origin',
+`Description` char(32) xpath='details')
+engine=CONNECT table_type=XML file_name='MYSQLTEST_VARDIR/tmp/beer.xml'
+tabname='table' option_list='rownode=tr,colnode=td%n';
+select * from beers;
+Name	Origin	Description
+NULL	NULL	NULL
+NULL	NULL	NULL
+drop table beers;
+create table beers (
+`Name` char(16) xpath='brandName',
+`Origin` char(16) xpath='origin',
+`Description` char(32) xpath='details')
+engine=CONNECT table_type=XML file_name='MYSQLTEST_VARDIR/tmp/beer.xml'
+tabname='table' option_list='rownode=tr,colnode=td';
+insert into beers values('11','22','33');
+drop table beers;
+execute immediate CONCAT('create table jsampall
+(Author char(128) jpath=''$.AUTHOR["', REPEAT('a',10000),'"]'')
+engine=CONNECT table_type=JSON
+file_name=''',REPLACE(@@secure_file_priv,'\\','/'),'tmp/test.json''');
+select  author from jsampall;
+author
+Jean-Christophe Bernadacaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa
+William J. Pardi
+drop table jsampall;
diff --git a/storage/connect/mysql-test/connect/t/misc.test b/storage/connect/mysql-test/connect/t/misc.test
new file mode 100644
index 00000000000..4dc8dded651
--- /dev/null
+++ b/storage/connect/mysql-test/connect/t/misc.test
@@ -0,0 +1,141 @@
+
+# Overlong table type
+--error ER_UNKNOWN_ERROR
+execute immediate concat('create table t engine=CONNECT table_type=JSON',REPEAT('1',5000),
+' FILE_NAME=''users.json'' HTTP=''http://localhost:4142'' URI=''/users''');
+
+# Overlong subtype
+--error ER_UNKNOWN_ERROR
+execute immediate concat('create table t  engine=CONNECT table_type=OEM module=''libname''
+Option_list=''Myopt=foo'' subtype=''MYTYPE',REPEAT('1', 10000), '''');
+
+
+# Overlong filename
+--error ER_UNKNOWN_ERROR
+execute immediate concat('create table t engine=CONNECT table_type=DBF file_name=''',
+ REPLACE(@@secure_file_priv,'\\','/'),'cust.dbf', REPEAT('1', 10000), '''');
+
+
+# Format string in SRCDEF
+--replace_result $MASTER_MYPORT MASTER_MYPORT
+eval create table t engine=connect table_type=mysql
+  CONNECTION='mysql://root@localhost:$MASTER_MYPORT/test/foobar'
+  SRCDEF='SELECT 1,''%n'' FROM DUAL WHERE %s';
+--error ER_GET_ERRMSG
+select *from t;
+drop table t;
+
+--replace_result $MASTER_MYPORT MASTER_MYPORT
+--error ER_UNKNOWN_ERROR
+eval create table t engine=connect table_type=mysql
+  CONNECTION='mysql://root@localhost:$MASTER_MYPORT/test/foobar'
+  SRCDEF='SELECT 1,%n FROM DUAL WHERE %s';
+
+--replace_result $MASTER_MYPORT MASTER_MYPORT
+eval create table t engine=connect table_type=mysql
+  CONNECTION='mysql://root@localhost:$MASTER_MYPORT/test/foobar'
+  SRCDEF='SELECT 1 FROM DUAL WHERE %s';
+select *from t;
+drop table t;
+
+write_file $MYSQLTEST_VARDIR/tmp/beer.xml;
+<?xml version="1.0"?>
+<Beers>
+  <table>
+    <th><td>Name</td><td>Origin</td><td>Description</td></th>
+    <tr>
+      <td><brandName>Huntsman</brandName></td>
+      <td><origin>Bath, UK</origin></td>
+      <td><details>Wonderful hop, light alcohol</details></td>
+    </tr>
+    <tr>
+      <td><brandName>Tuborg</brandName></td>
+      <td><origin>Danmark</origin></td>
+      <td><details>In small bottles</details></td>
+    </tr>
+  </table>
+</Beers>
+EOF
+
+--replace_result $MYSQLTEST_VARDIR MYSQLTEST_VARDIR
+# Format string in colnode
+eval create table beers (
+`Name` char(16) xpath='brandName',
+`Origin` char(16) xpath='origin',
+`Description` char(32) xpath='details')
+engine=CONNECT table_type=XML file_name='$MYSQLTEST_VARDIR/tmp/beer.xml'
+tabname='table' option_list='rownode=tr,colnode=td%n';
+select * from beers;
+drop table beers;
+
+--replace_result $MYSQLTEST_VARDIR MYSQLTEST_VARDIR
+eval create table beers (
+`Name` char(16) xpath='brandName',
+`Origin` char(16) xpath='origin',
+`Description` char(32) xpath='details')
+engine=CONNECT table_type=XML file_name='$MYSQLTEST_VARDIR/tmp/beer.xml'
+tabname='table' option_list='rownode=tr,colnode=td';
+insert into beers values('11','22','33');
+drop table beers;
+
+remove_file $MYSQLTEST_VARDIR/tmp/beer.xml;
+
+write_file  $MYSQLTEST_VARDIR/tmp/test.json;
+[
+  {
+    "ISBN": "9782212090819",
+    "LANG": "fr",
+    "SUBJECT": "applications",
+    "AUTHOR": [
+      {
+        "FIRSTNAME": "Jean-Christophe",
+        "LASTNAME": "Bernadac"
+      },
+      {
+        "FIRSTNAME": "François",
+        "LASTNAME": "Knab"
+      }
+    ],
+    "TITLE": "Construire une application XML",
+    "PUBLISHER": {
+      "NAME": "Eyrolles",
+      "PLACE": "Paris"
+    },
+    "DATEPUB": 1999
+  },
+  {
+    "ISBN": "9782840825685",
+    "LANG": "fr",
+    "SUBJECT": "applications",
+    "AUTHOR": [
+      {
+        "FIRSTNAME": "William J.",
+        "LASTNAME": "Pardi"
+      }
+    ],
+    "TITLE": "XML en Action",
+    "TRANSLATED": {
+       "PREFIX": "adapté de l'anglais par",
+       "TRANSLATOR": {
+          "FIRSTNAME": "James",
+        "LASTNAME": "Guerin"
+        }
+    },
+    "PUBLISHER": {
+      "NAME": "Microsoft Press",
+      "PLACE": "Paris"
+    },
+    "DATEPUB": 1999
+  }
+]
+EOF
+
+execute immediate CONCAT('create table jsampall
+(Author char(128) jpath=''$.AUTHOR["', REPEAT('a',10000),'"]'')
+engine=CONNECT table_type=JSON
+file_name=''',REPLACE(@@secure_file_priv,'\\','/'),'tmp/test.json''');
+
+select  author from jsampall;
+drop table jsampall;
+remove_file $MYSQLTEST_VARDIR/tmp/test.json;
+
diff --git a/storage/connect/plugutil.cpp b/storage/connect/plugutil.cpp
index 4aecbadfc6a..db62ad5bb2c 100644
--- a/storage/connect/plugutil.cpp
+++ b/storage/connect/plugutil.cpp
@@ -259,6 +259,12 @@ LPCSTR PlugSetPath(LPSTR pBuff, LPCSTR prefix, LPCSTR FileName, LPCSTR defpath)
 	if (trace(2))
 		htrc("prefix=%s fn=%s path=%s\n", prefix, FileName, defpath);
 
+	if (strlen(FileName) >= _MAX_PATH)
+	{
+		*pBuff= 0; /* Hope this is treated as error of some kind*/
+		return FileName;
+	}
+
   if (!strncmp(FileName, "//", 2) || !strncmp(FileName, "\\\\", 2)) {
     strcpy(pBuff, FileName);       // Remote file
     return pBuff;
diff --git a/storage/connect/reldef.cpp b/storage/connect/reldef.cpp
index d7715a2ea9f..8be3a013e8c 100644
--- a/storage/connect/reldef.cpp
+++ b/storage/connect/reldef.cpp
@@ -93,7 +93,12 @@ PQRYRES OEMColumns(PGLOBAL g, PTOS topt, char* tab, char* db, bool info)
 	if (check_valid_path(module, strlen(module))) {
 		strcpy(g->Message, "Module cannot contain a path");
 		return NULL;
-	} else
+	}
+	else if (strlen(subtype)+1+3 >= sizeof(getname)) {
+		strcpy(g->Message, "Subtype string too long");
+		return NULL;
+	}
+	else
 		PlugSetPath(soname, module, GetPluginDir());
 
 	// The exported name is always in uppercase
diff --git a/storage/connect/tabbson.cpp b/storage/connect/tabbson.cpp
index 8569e39f678..59d2b7ed1b0 100644
--- a/storage/connect/tabbson.cpp
+++ b/storage/connect/tabbson.cpp
@@ -1788,7 +1788,7 @@ bool BSONCOL::SetArrayOptions(PGLOBAL g, char* p, int i, PSZ nm)
       p[--n] = 0;
     } else if (!IsNum(p)) {
       // Wrong array specification
-      sprintf(g->Message, "Invalid array specification %s for %s", p, Name);
+      snprintf(g->Message, sizeof(g->Message), "Invalid array specification %s for %s", p, Name);
       return true;
     } // endif p
 
diff --git a/storage/connect/tabext.cpp b/storage/connect/tabext.cpp
index 9e8154cd233..1e0f6254f87 100644
--- a/storage/connect/tabext.cpp
+++ b/storage/connect/tabext.cpp
@@ -286,6 +286,37 @@ int TDBEXT::Decode(PCSZ txt, char *buf, size_t n)
 	return 0;
 } // end of Decode
 
+/*
+  Count number of %s placeholders in string.
+  Returns -1 if other sprintf placeholders are found, .g %d
+*/
+static int count_placeholders(const char *fmt)
+{
+  int cnt= 0;
+  for (const char *p=fmt; *p; p++)
+  {
+    if (*p == '%')
+    {
+      switch (p[1])
+      {
+      case 's':
+        /* %s found */
+        cnt++;
+        p++;
+        break;
+      case '%':
+        /* masking char for % found */
+        p++;
+        break;
+      default:
+        /* some other placeholder found */
+        return -1;
+      }
+    }
+  }
+  return cnt;
+}
+
 /***********************************************************************/
 /*  MakeSrcdef: make the SQL statement from SRDEF option.              */
 /***********************************************************************/
@@ -310,16 +341,29 @@ bool TDBEXT::MakeSrcdef(PGLOBAL g)
 				? To_CondFil->Having : PlugDup(g, "1=1");
 		} // endif ph
 
-		if (!stricmp(ph, "W")) {
+		int n_placeholders = count_placeholders(Srcdef);
+		if (n_placeholders < 0)
+		{
+			strcpy(g->Message, "MakeSQL: Wrong place holders specification");
+			return true;
+		}
+
+		if (!stricmp(ph, "W") && n_placeholders <= 1) {
 			Query = new(g)STRING(g, strlen(Srcdef) + strlen(fil1));
 			Query->SetLength(sprintf(Query->GetStr(), Srcdef, fil1));
-		} else if (!stricmp(ph, "WH")) {
+		}
+		else if (!stricmp(ph, "WH") && n_placeholders <= 2)
+		{
 			Query = new(g)STRING(g, strlen(Srcdef) + strlen(fil1) + strlen(fil2));
 			Query->SetLength(sprintf(Query->GetStr(), Srcdef, fil1, fil2));
-		} else if (!stricmp(ph, "H")) {
+		}
+		else if (!stricmp(ph, "H") && n_placeholders <= 1)
+		{
 			Query = new(g)STRING(g, strlen(Srcdef) + strlen(fil2));
 			Query->SetLength(sprintf(Query->GetStr(), Srcdef, fil2));
-		} else if (!stricmp(ph, "HW")) {
+		}
+		else if (!stricmp(ph, "HW") && n_placeholders <= 2)
+		{
 			Query = new(g)STRING(g, strlen(Srcdef) + strlen(fil1) + strlen(fil2));
 			Query->SetLength(sprintf(Query->GetStr(), Srcdef, fil2, fil1));
 		} else {
diff --git a/storage/connect/tabjson.cpp b/storage/connect/tabjson.cpp
index 7eff0a68c4b..8cd452ee570 100644
--- a/storage/connect/tabjson.cpp
+++ b/storage/connect/tabjson.cpp
@@ -1384,7 +1384,7 @@ bool JSONCOL::SetArrayOptions(PGLOBAL g, char *p, int i, PSZ nm)
       p[--n] = 0;
     } else if (!IsNum(p)) {
       // Wrong array specification
-      sprintf(g->Message, "Invalid array specification %s for %s", p, Name);
+      snprintf(g->Message, sizeof(g->Message), "Invalid array specification %s for %s", p, Name);
       return true;
     } // endif p
 
diff --git a/storage/connect/tabmysql.cpp b/storage/connect/tabmysql.cpp
index f8f995f211e..a701db1e8e9 100644
--- a/storage/connect/tabmysql.cpp
+++ b/storage/connect/tabmysql.cpp
@@ -904,6 +904,11 @@ bool TDBMYSQL::OpenDB(PGLOBAL g)
   /*********************************************************************/
   if (Mode == MODE_READ || Mode == MODE_READX) {
     MakeSelect(g, Mode == MODE_READX);
+    if (Mode == MODE_READ && !Query)
+    {
+      Myc.Close();
+      return true;
+    }
     m_Rc = (Mode == MODE_READ)
          ? Myc.ExecSQL(g, Query->GetStr()) : RC_OK;
 
diff --git a/storage/connect/tabxml.cpp b/storage/connect/tabxml.cpp
index cb428f5a94c..7357d2373c8 100644
--- a/storage/connect/tabxml.cpp
+++ b/storage/connect/tabxml.cpp
@@ -1511,9 +1511,13 @@ bool XMLCOL::ParseXpath(PGLOBAL g, bool mode)
   if (!mode)
     // Take care of an eventual extra column node a la html
     if (Tdbp->Colname) {
-      sprintf(pbuf, Tdbp->Colname, Rank + ((Tdbp->Usedom) ? 0 : 1));
-      strcat(pbuf, "/");
-      } // endif Colname
+      char *p = strstr(Tdbp->Colname, "%d");
+      if (p)
+        snprintf(pbuf, len + 3, "%.*s%d%s/", (int) (p - Tdbp->Colname), Tdbp->Colname,
+            Rank + (Tdbp->Usedom ? 0 : 1), p + 2);
+      else
+        snprintf(pbuf, len + 3, "%s/", Tdbp->Colname);
+    } // endif Colname
 
   if (Xname) {
     if (Type == 2) {