Harden SonarCloud S5801/S5813 string handling hotspots

include/Admin_ifaces.h

@@ -44,6 +44,49 @@ class ifaces_desc {
 
 class admin_main_loop_listeners {
 	private:
+	struct tokenizer_owner {
+		tokenizer_t value;
+
+		explicit tokenizer_owner(char *list) {
+			tokenizer(&value, list, ";", TOKENIZER_NO_EMPTIES);
+		}
+
+		~tokenizer_owner() {
+			free_tokenizer(&value);
+		}
+
+		tokenizer_owner(const tokenizer_owner&) = delete;
+		tokenizer_owner& operator=(const tokenizer_owner&) = delete;
+		tokenizer_owner(tokenizer_owner&&) = delete;
+		tokenizer_owner& operator=(tokenizer_owner&&) = delete;
+	};
+
+	struct interface_array_owner {
+		char **value;
+
+		explicit interface_array_owner(char **value_) : value(value_) {}
+
+		~interface_array_owner() {
+			if (value) {
+				for (int i = 0; i < MAX_IFACES; ++i) {
+					l_free(0, value[i]);
+				}
+				l_free(0, value);
+			}
+		}
+
+		interface_array_owner(const interface_array_owner&) = delete;
+		interface_array_owner& operator=(const interface_array_owner&) = delete;
+		interface_array_owner(interface_array_owner&&) = delete;
+		interface_array_owner& operator=(interface_array_owner&&) = delete;
+
+		char **release() {
+			char **released = value;
+			value = nullptr;
+			return released;
+		}
+	};
+
 	int version;
 #ifdef PA_PTHREAD_MUTEX
 	pthread_rwlock_t rwlock;
@@ -129,19 +172,32 @@ class admin_main_loop_listeners {
 
 	bool update_ifaces(char *list, char ***_ifaces) {
 		wrlock();
-		int i;
-		char **ifaces=*_ifaces;
-		tokenizer_t tok;
-		tokenizer( &tok, list, ";", TOKENIZER_NO_EMPTIES );
-		const char* token;
-		ifaces=reset_ifaces(ifaces);
-		i=0;
-		for ( token = tokenize( &tok ) ; token && i < MAX_IFACES ; token = tokenize( &tok ) ) {
-			ifaces[i]=(char *)malloc(strlen(token)+1);
-			strcpy(ifaces[i],token);
+		char **old_ifaces = *_ifaces;
+		char **new_ifaces = (char **)l_alloc(MAX_IFACES * sizeof(char *));
+		if (new_ifaces != nullptr) {
+			memset(new_ifaces, 0, MAX_IFACES * sizeof(char *));
+		}
+		interface_array_owner replacement(new_ifaces);
+		if (replacement.value == nullptr) {
+			wrunlock();
+			return false;
+		}
+
+		tokenizer_owner tokens(list);
+		int i = 0;
+		for (const char *token = tokenize(&tokens.value);
+			token && i < MAX_IFACES;
+			token = tokenize(&tokens.value)) {
+			replacement.value[i] = strdup(token);
+			if (replacement.value[i] == nullptr) {
+				wrunlock();
+				return false;
+			}
 			i++;
 		}
-		free_tokenizer( &tok );
+
+		interface_array_owner previous(old_ifaces);
+		*_ifaces = replacement.release();
 		version++;
 		wrunlock();
 		return true;

include/MySQL_Query_Processor.h

@@ -60,7 +60,7 @@ class MySQL_Query_Processor : public Query_Processor<MySQL_Query_Processor> {
 
 private:
 	Command_Counter* commands_counters[MYSQL_COM_QUERY___NONE];
-	static bool _is_valid_gtid(char* gtid, size_t gtid_len);
+	static bool _is_valid_gtid(const char* gtid, size_t gtid_len);
 	static MySQL_Query_Processor_Rule_t* new_query_rule(const MySQL_Query_Processor_Rule_t* mqr);
 
 	inline
@@ -74,25 +74,29 @@ class MySQL_Query_Processor : public Query_Processor<MySQL_Query_Processor> {
 
 	inline
 	void query_parser_first_comment_extended(const char* key, const char* value, MySQL_Query_Processor_Output* qpo) {
-		if (!strcasecmp(key, "min_gtid")) {
-			if (mysql_thread___ignore_min_gtid_annotations) {
-				proxy_debug(PROXY_DEBUG_MYSQL_QUERY_PROCESSOR, 5, "Ignoring min_gtid=%s\n", value);
-			} else {
-				size_t l = strlen(value);
-				if (_is_valid_gtid((char*)value, l)) {
-					char* buf = (char*)malloc(l + 1);
-					strncpy(buf, value, l);
-					buf[l] = '\0';
-
-					if (qpo->min_gtid) {
-						free(qpo->min_gtid);
-					}
-					qpo->min_gtid = buf;
-				} else {
-					proxy_warning("Invalid min_gtid value=%s\n", value);
-				}
-			}
+		if (strcasecmp(key, "min_gtid")) {
+			return;
+		}
+		if (mysql_thread___ignore_min_gtid_annotations) {
+			proxy_debug(PROXY_DEBUG_MYSQL_QUERY_PROCESSOR, 5, "Ignoring min_gtid=%s\n", value);
+			return;
+		}
+		size_t l = strlen(value);
+		if (!_is_valid_gtid(value, l)) {
+			proxy_warning("Invalid min_gtid value=%s\n", value);
+			return;
+		}
+		char* buf = (char*)l_alloc(l + 1);
+		if (buf == nullptr) {
+			proxy_warning("Unable to allocate memory for min_gtid=%s\n", value);
+			return;
+		}
+		memcpy(buf, value, l);
+		buf[l] = '\0';
+		if (qpo->min_gtid) {
+			l_free(0, qpo->min_gtid);
 		}
+		qpo->min_gtid = buf;
 	}
 
 	friend class Query_Processor;

include/proxysql_glovars.hpp

@@ -48,9 +48,12 @@ class ProxySQL_Checksum_Value {
 		epoch = 0;
 		in_shutdown = false;
 	}
-	void set_checksum(char *c) {
+	void set_checksum(const char *c) {
 		memset(checksum,0,ProxySQL_Checksum_Value_LENGTH);
-		strncpy(checksum,c,ProxySQL_Checksum_Value_LENGTH);
+		if (c) {
+			const size_t length = strnlen(c, ProxySQL_Checksum_Value_LENGTH);
+			memcpy(checksum, c, length);
+		}
 		replace_checksum_zeros(checksum);
 	}
 	~ProxySQL_Checksum_Value() {

include/proxysql_macros.h

@@ -4,7 +4,7 @@
 #define char_malloc (char *)malloc
 #define free_null(__c) { if(__c) { free(__c); (__c)=NULL; } }
 
-#define itostr(__s, __i)  { (__s)=char_malloc(32); sprintf(__s, "%lld", (__i)); }
+#define itostr(__s, __i)  { (__s)=char_malloc(32); snprintf((__s), 32, "%lld", (__i)); }
 
 // fast memory copy forward . Use this instead of memcpy for small buffers
 #define MEM_COPY_FWD(dst_p, src_p, bytes)  \