Merge pull request #4455 from sysown/v2.x_memleak_fix_stats_mysql_pre…

…pared_statements_info Fixed memory leak issue in querying of stats_mysql_prepared_statements_info table + Resolved a heap use-after-free issue
sysown · Feb 20, 2024 · 6a6807c · 6a6807c
2 parents afaf642 + 219fc1b
commit 6a6807c
Show file tree

Hide file tree

Showing 6 changed files with 152 additions and 139 deletions.
diff --git a/include/ProxySQL_HTTP_Server.hpp b/include/ProxySQL_HTTP_Server.hpp
@@ -11,9 +11,9 @@ class ProxySQL_HTTP_Server {
 	time_t cur_time;
 	pthread_mutex_t check_version_mutex;
 	time_t last_check_version;
-	std::string * generate_header(char *);
-	std::string * generate_canvas(char *);
-	std::string * generate_chart(char *chart_name, char *ts, int nsets, char **dname, char **llabel, char **values);
+	std::string generate_header(char *);
+	std::string generate_canvas(char *);
+	std::string generate_chart(char *chart_name, char *ts, int nsets, char **dname, char **llabel, char **values);
 	char *extract_values(SQLite3_result *result, int idx, bool relative, double mult=1);
 	char *extract_ts(SQLite3_result *result, bool relative);
 	public:

diff --git a/lib/MySQL_PreparedStatement.cpp b/lib/MySQL_PreparedStatement.cpp
@@ -16,6 +16,8 @@
 extern MySQL_STMT_Manager_v14 *GloMyStmt;
 //#endif
 
+const int PS_GLOBAL_STATUS_FIELD_NUM = 9;
+
 static uint64_t stmt_compute_hash(char *user,
                                   char *schema, char *query,
                                   unsigned int query_length) {
@@ -1035,7 +1037,7 @@ class PS_global_stats {
 	}
 	char **get_row() {
 		char buf[128];
-		char **pta=(char **)malloc(sizeof(char *)*9);
+		char **pta=(char **)malloc(sizeof(char *)*PS_GLOBAL_STATUS_FIELD_NUM);
 		sprintf(buf,"%lu",statement_id);
 		pta[0]=strdup(buf);
 		assert(schemaname);
@@ -1061,7 +1063,7 @@ class PS_global_stats {
 	}
 	void free_row(char **pta) {
 		int i;
-		for (i=0;i<7;i++) {
+		for (i=0;i<PS_GLOBAL_STATUS_FIELD_NUM;i++) {
 			assert(pta[i]);
 			free(pta[i]);
 		}
@@ -1072,7 +1074,7 @@ class PS_global_stats {
 
 SQLite3_result * MySQL_STMT_Manager_v14::get_prepared_statements_global_infos() {
 	proxy_debug(PROXY_DEBUG_MYSQL_QUERY_PROCESSOR, 4, "Dumping current prepared statements global info\n");
-	SQLite3_result *result=new SQLite3_result(9);
+	SQLite3_result *result=new SQLite3_result(PS_GLOBAL_STATUS_FIELD_NUM);
 	rdlock();
 	result->add_column_definition(SQLITE_TEXT,"stmt_id");
 	result->add_column_definition(SQLITE_TEXT,"schemaname");

diff --git a/lib/MySQL_Thread.cpp b/lib/MySQL_Thread.cpp
@@ -1809,16 +1809,18 @@ bool MySQL_Threads_Handler::set_variable(char *name, const char *value) {	// thi
 			return false;
 		} else if (value[0] == '/') {
 			char *full_path = strdup(value);
-                        char *eval_dirname = dirname(full_path);
-                        DIR* eventlog_dir = opendir(eval_dirname);
-			free(full_path);
-                        if (eventlog_dir) {
+			char* eval_dirname = dirname(full_path);
+			DIR* eventlog_dir = opendir(eval_dirname);
+			if (eventlog_dir) {
 				closedir(eventlog_dir);
 				free(variables.auditlog_filename);
-				variables.auditlog_filename=strdup(value);
-                                return true;
-			} else {
+				variables.auditlog_filename = strdup(value);
+				free(full_path);
+				return true;
+			}
+			else {
 				proxy_error("%s is an invalid value for auditlog_filename path, the directory cannot be accessed\n", eval_dirname);
+				free(full_path);
 				return false;
 			}
 		} else {
@@ -1833,16 +1835,18 @@ bool MySQL_Threads_Handler::set_variable(char *name, const char *value) {	// thi
 			return false;
 		} else if (value[0] == '/') {
 			char *full_path = strdup(value);
-                        char *eval_dirname = dirname(full_path);
-                        DIR* eventlog_dir = opendir(eval_dirname);
-			free(full_path);
-                        if (eventlog_dir) {
+			char* eval_dirname = dirname(full_path);
+			DIR* eventlog_dir = opendir(eval_dirname);
+			if (eventlog_dir) {
 				closedir(eventlog_dir);
 				free(variables.eventslog_filename);
-				variables.eventslog_filename=strdup(value);
-                                return true;
-			} else {
+				variables.eventslog_filename = strdup(value);
+				free(full_path);
+				return true;
+			}
+			else {
 				proxy_error("%s is an invalid value for eventslog_filename path, the directory cannot be accessed\n", eval_dirname);
+				free(full_path);
 				return false;
 			}
 		} else {

diff --git a/lib/ProxySQL_Admin.cpp b/lib/ProxySQL_Admin.cpp
@@ -7007,6 +7007,12 @@ ProxySQL_Admin::~ProxySQL_Admin() {
 	delete (RE2 *)match_regexes.re[3];
 	free(match_regexes.re);
 	delete (re2::RE2::Options *)match_regexes.opt;
+
+	for (std::unordered_map<std::string, void*>::iterator it = map_test_mysql_firewall_whitelist_rules.begin(); it != map_test_mysql_firewall_whitelist_rules.end(); ++it) {
+		PtrArray* myptrarray = (PtrArray*)it->second;
+		delete myptrarray;
+	}
+	map_test_mysql_firewall_whitelist_rules.clear();
 };
 
 // This function is used only used to export what collations are available