From 1e4c10865992bcf6ed9666a3b64caa5edaf68a5c Mon Sep 17 00:00:00 2001 From: Rahim Kanji Date: Fri, 16 Feb 2024 11:52:10 +0500 Subject: [PATCH 1/6] Fixed memory leak in querying stats_mysql_prepared_statements_info --- lib/MySQL_PreparedStatement.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/MySQL_PreparedStatement.cpp b/lib/MySQL_PreparedStatement.cpp index f3d82ea084..5f7719492a 100644 --- a/lib/MySQL_PreparedStatement.cpp +++ b/lib/MySQL_PreparedStatement.cpp @@ -1061,7 +1061,7 @@ class PS_global_stats { } void free_row(char **pta) { int i; - for (i=0;i<7;i++) { + for (i=0;i<9;i++) { assert(pta[i]); free(pta[i]); } From e53cd200ab1dbd8f3be7e736fca77f1e568600ca Mon Sep 17 00:00:00 2001 From: Rahim Kanji Date: Fri, 16 Feb 2024 12:08:24 +0500 Subject: [PATCH 2/6] Code cleanup --- lib/MySQL_PreparedStatement.cpp | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/lib/MySQL_PreparedStatement.cpp b/lib/MySQL_PreparedStatement.cpp index 5f7719492a..7435a69f9f 100644 --- 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<9;i++) { + for (i=0;iadd_column_definition(SQLITE_TEXT,"stmt_id"); result->add_column_definition(SQLITE_TEXT,"schemaname"); From 3f7a90ce97f225d8cd6dbd450c8b290f59cdb36b Mon Sep 17 00:00:00 2001 From: Rahim Kanji Date: Fri, 16 Feb 2024 13:11:56 +0500 Subject: [PATCH 3/6] Fixed heap use after free issue. --- lib/MySQL_Thread.cpp | 32 ++++++++++++++++++-------------- 1 file changed, 18 insertions(+), 14 deletions(-) diff --git a/lib/MySQL_Thread.cpp b/lib/MySQL_Thread.cpp index a7965a428d..ff84dc024a 100644 --- 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 { From 4c1974ce080a2f86ab13fc4c1244b01261cc7a9a Mon Sep 17 00:00:00 2001 From: Rahim Kanji Date: Fri, 16 Feb 2024 16:36:20 +0500 Subject: [PATCH 4/6] Fixed memory leaks in ProxySQL HTTP Server --- include/ProxySQL_HTTP_Server.hpp | 6 +- lib/ProxySQL_HTTP_Server.cpp | 231 ++++++++++++++++--------------- 2 files changed, 119 insertions(+), 118 deletions(-) diff --git a/include/ProxySQL_HTTP_Server.hpp b/include/ProxySQL_HTTP_Server.hpp index 342af02007..7d8a28df58 100644 --- 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/ProxySQL_HTTP_Server.cpp b/lib/ProxySQL_HTTP_Server.cpp index e82b18a249..ff38885582 100644 --- a/lib/ProxySQL_HTTP_Server.cpp +++ b/lib/ProxySQL_HTTP_Server.cpp @@ -505,23 +505,23 @@ int ProxySQL_HTTP_Server::handler(void *cls, struct MHD_Connection *connection, } */ if (strcmp(valmetric,"system")==0) { - string *s = generate_header((char *)"ProxySQL Graphs"); + string s = generate_header((char *)"ProxySQL Graphs"); char *buttons = generate_buttons((char *)"system"); - s->append(buttons); + s.append(buttons); free(buttons); - s->append("
\n"); - string *s1 = generate_canvas((char *)"myChart1"); - s->append(s1->c_str()); - s->append("

\n"); + s.append("
\n"); + string s1 = generate_canvas((char *)"myChart1"); + s.append(s1.c_str()); + s.append("

\n"); s1 = generate_canvas((char *)"myChart2"); - s->append(s1->c_str()); - s->append("

\n"); + s.append(s1.c_str()); + s.append("

\n"); s1 = generate_canvas((char *)"myChart3"); - s->append(s1->c_str()); - s->append("

\n"); + s.append(s1.c_str()); + s.append("

\n"); s1 = generate_canvas((char *)"myChart4"); - s->append(s1->c_str()); - s->append("
\n"); + s.append(s1.c_str()); + s.append("
\n"); SQLite3_result *cpu_sqlite = GloProxyStats->get_system_cpu_metrics(interval_i); #ifndef NOJEM SQLite3_result *memory_sqlite = GloProxyStats->get_system_memory_metrics(interval_i); @@ -542,14 +542,13 @@ int ProxySQL_HTTP_Server::handler(void *cls, struct MHD_Connection *connection, nv[1] = extract_values(cpu_sqlite,3,true,(double)1/sysconf(_SC_CLK_TCK)); ts = extract_ts(cpu_sqlite,true); s1 = generate_chart((char *)"myChart1",ts,2,nm,nl,nv); - s->append(s1->c_str()); + s.append(s1.c_str()); free(nm); free(nl); free(nv[0]); free(nv[1]); free(nv); free(ts); - #ifndef NOJEM nm = (char **)malloc(sizeof(char *)*5); @@ -572,7 +571,7 @@ int ProxySQL_HTTP_Server::handler(void *cls, struct MHD_Connection *connection, nv[4] = extract_values(memory_sqlite,6,false); ts = extract_ts(cpu_sqlite,true); s1 = generate_chart((char *)"myChart2",ts,5,nm,nl,nv); - s->append(s1->c_str()); + s.append(s1.c_str()); free(nm); free(nl); free(nv[0]); @@ -582,33 +581,35 @@ int ProxySQL_HTTP_Server::handler(void *cls, struct MHD_Connection *connection, free(nv[4]); free(nv); free(ts); + delete memory_sqlite; #endif + delete cpu_sqlite; - s->append(""); - response = MHD_create_response_from_buffer(s->length(), (void *) s->c_str(), MHD_RESPMEM_PERSISTENT); + s.append(""); + response = MHD_create_response_from_buffer(s.length(), (void *) s.c_str(), MHD_RESPMEM_PERSISTENT); ret = MHD_queue_response (connection, MHD_HTTP_OK, response); MHD_destroy_response (response); return ret; } if (strcmp(valmetric,"mysql")==0) { - string *s = generate_header((char *)"ProxySQL Graphs"); + string s = generate_header((char *)"ProxySQL Graphs"); char *buttons = generate_buttons((char *)"mysql"); - s->append(buttons); + s.append(buttons); free(buttons); - s->append("
\n"); - string *s1 = generate_canvas((char *)"myChart1"); - s->append(s1->c_str()); - s->append("

\n"); + s.append("
\n"); + string s1 = generate_canvas((char *)"myChart1"); + s.append(s1.c_str()); + s.append("

\n"); s1 = generate_canvas((char *)"myChart2"); - s->append(s1->c_str()); - s->append("

\n"); + s.append(s1.c_str()); + s.append("

\n"); s1 = generate_canvas((char *)"myChart3"); - s->append(s1->c_str()); - s->append("

\n"); + s.append(s1.c_str()); + s.append("

\n"); s1 = generate_canvas((char *)"myChart4"); - s->append(s1->c_str()); - s->append("
\n"); + s.append(s1.c_str()); + s.append("
\n"); char **nm = NULL; char **nl = NULL; char **nv = NULL; @@ -638,7 +639,7 @@ int ProxySQL_HTTP_Server::handler(void *cls, struct MHD_Connection *connection, nv[5] = extract_values(mysql_metrics_sqlite,7,true,(double)1); ts = extract_ts(mysql_metrics_sqlite,true); s1 = generate_chart((char *)"myChart1",ts,6,nm,nl,nv); - s->append(s1->c_str()); + s.append(s1.c_str()); free(nm); free(nl); for (int aa=0 ; aa<6 ; aa++) { @@ -671,7 +672,7 @@ int ProxySQL_HTTP_Server::handler(void *cls, struct MHD_Connection *connection, nv[5] = extract_values(mysql_metrics_sqlite,13,true); ts = extract_ts(mysql_metrics_sqlite,true); s1 = generate_chart((char *)"myChart2",ts,6,nm,nl,nv); - s->append(s1->c_str()); + s.append(s1.c_str()); free(nm); free(nl); for (int aa=0 ; aa<6 ; aa++) { @@ -679,6 +680,7 @@ int ProxySQL_HTTP_Server::handler(void *cls, struct MHD_Connection *connection, } free(nv); free(ts); + delete mysql_metrics_sqlite; SQLite3_result *myhgm_metrics_sqlite = GloProxyStats->get_myhgm_metrics(interval_i); @@ -702,7 +704,7 @@ int ProxySQL_HTTP_Server::handler(void *cls, struct MHD_Connection *connection, nv[4] = extract_values(myhgm_metrics_sqlite,6,true); ts = extract_ts(myhgm_metrics_sqlite,true); s1 = generate_chart((char *)"myChart3",ts,5,nm,nl,nv); - s->append(s1->c_str()); + s.append(s1.c_str()); free(nm); free(nl); for (int aa=0 ; aa<5 ; aa++) { @@ -710,34 +712,34 @@ int ProxySQL_HTTP_Server::handler(void *cls, struct MHD_Connection *connection, } free(nv); free(ts); + delete myhgm_metrics_sqlite; - s->append(""); - response = MHD_create_response_from_buffer(s->length(), (void *) s->c_str(), MHD_RESPMEM_MUST_COPY); + s.append(""); + response = MHD_create_response_from_buffer(s.length(), (void *) s.c_str(), MHD_RESPMEM_MUST_COPY); ret = MHD_queue_response (connection, MHD_HTTP_OK, response); MHD_destroy_response (response); - delete s; return ret; } if (strcmp(valmetric,"cache")==0) { - string *s = generate_header((char *)"ProxySQL Graphs"); + string s = generate_header((char *)"ProxySQL Graphs"); char *buttons = generate_buttons((char *)"cache"); - s->append(buttons); + s.append(buttons); free(buttons); - s->append("
\n"); - string *s1 = generate_canvas((char *)"myChart1"); - s->append(s1->c_str()); - s->append("

\n"); + s.append("
\n"); + string s1 = generate_canvas((char *)"myChart1"); + s.append(s1.c_str()); + s.append("

\n"); s1 = generate_canvas((char *)"myChart2"); - s->append(s1->c_str()); - s->append("

\n"); + s.append(s1.c_str()); + s.append("

\n"); s1 = generate_canvas((char *)"myChart3"); - s->append(s1->c_str()); - s->append("

\n"); + s.append(s1.c_str()); + s.append("

\n"); s1 = generate_canvas((char *)"myChart4"); - s->append(s1->c_str()); - s->append("
\n"); + s.append(s1.c_str()); + s.append("
\n"); SQLite3_result *mysql_metrics_sqlite = GloProxyStats->get_MySQL_Query_Cache_metrics(interval_i); char **nm = NULL; char **nl = NULL; @@ -764,7 +766,7 @@ int ProxySQL_HTTP_Server::handler(void *cls, struct MHD_Connection *connection, nv[4] = extract_values(mysql_metrics_sqlite,8,false,(double)1); ts = extract_ts(mysql_metrics_sqlite,true); s1 = generate_chart((char *)"myChart1",ts,5,nm,nl,nv); - s->append(s1->c_str()); + s.append(s1.c_str()); free(nm); free(nl); for (int aa=0 ; aa<5 ; aa++) { @@ -788,7 +790,7 @@ int ProxySQL_HTTP_Server::handler(void *cls, struct MHD_Connection *connection, nv[2] = extract_values(mysql_metrics_sqlite,9,false,(double)1/1024/1024); ts = extract_ts(mysql_metrics_sqlite,true); s1 = generate_chart((char *)"myChart2",ts,3,nm,nl,nv); - s->append(s1->c_str()); + s.append(s1.c_str()); free(nm); free(nl); for (int aa=0 ; aa<3 ; aa++) { @@ -796,12 +798,12 @@ int ProxySQL_HTTP_Server::handler(void *cls, struct MHD_Connection *connection, } free(nv); free(ts); + delete mysql_metrics_sqlite; - s->append(""); - response = MHD_create_response_from_buffer(s->length(), (void *) s->c_str(), MHD_RESPMEM_MUST_COPY); + s.append(""); + response = MHD_create_response_from_buffer(s.length(), (void *) s.c_str(), MHD_RESPMEM_MUST_COPY); ret = MHD_queue_response (connection, MHD_HTTP_OK, response); MHD_destroy_response (response); - delete s; return ret; } } @@ -825,15 +827,14 @@ int ProxySQL_HTTP_Server::handler(void *cls, struct MHD_Connection *connection, return ret; } if (strcmp(url,"/")==0) { - string *s = generate_header((char *)"ProxySQL Home"); + string s = generate_header((char *)"ProxySQL Home"); char *home = generate_home(); - s->append(home); + s.append(home); free(home); - s->append(""); - response = MHD_create_response_from_buffer(s->length(), (void *) s->c_str(), MHD_RESPMEM_MUST_COPY); + s.append(""); + response = MHD_create_response_from_buffer(s.length(), (void *) s.c_str(), MHD_RESPMEM_MUST_COPY); ret = MHD_queue_response (connection, MHD_HTTP_OK, response); MHD_destroy_response (response); - delete s; return ret; } response = MHD_create_response_from_buffer (strlen (EMPTY_PAGE), (void *) EMPTY_PAGE, MHD_RESPMEM_PERSISTENT); @@ -843,24 +844,24 @@ int ProxySQL_HTTP_Server::handler(void *cls, struct MHD_Connection *connection, return ret; } -string * ProxySQL_HTTP_Server::generate_header(char *s) { - string *a = new string(); - a->append("\n"); - a->append(s); - a->append("\n"); - a->append("\n"); - a->append("\n"); - a->append("\n"); - a->append("\n\n\n"); - a->append("
\nProxySQL\n"); +string ProxySQL_HTTP_Server::generate_header(char *s) { + string a{}; + a.append("\n"); + a.append(s); + a.append("\n"); + a.append("\n"); + a.append("\n"); + a.append("\n"); + a.append("\n\n\n"); + a.append("
\nProxySQL\n"); return a; } -string * ProxySQL_HTTP_Server::generate_canvas(char *s) { - string *a = new string(); - a->append("
append(s); - a->append("\" width=\"700\" height=\"330\">
\n"); +string ProxySQL_HTTP_Server::generate_canvas(char *s) { + string a{}; + a.append("
\n"); return a; } ProxySQL_HTTP_Server::ProxySQL_HTTP_Server() { @@ -878,61 +879,61 @@ ProxySQL_HTTP_Server::~ProxySQL_HTTP_Server() { } } -string * ProxySQL_HTTP_Server::generate_chart(char *chart_name, char *ts, int nsets, char **dname, char **llabel, char **values) { +string ProxySQL_HTTP_Server::generate_chart(char *chart_name, char *ts, int nsets, char **dname, char **llabel, char **values) { char *h=(char *)"0123456789abcdef"; - string *ret= new string(); + string ret{}; int i; - ret->append("\n"); + ret.append(" ]\n"); + ret.append(" },\n"); + ret.append("options: {\n"); + ret.append(" scales: {\n"); + ret.append(" xAxes: [{\n"); + ret.append(" ticks: {\n"); +// ret.append(" autoSkip: true,\n"); +// ret.append(" maxRotation: 0,\n"); +// ret.append(" minRotation: 0\n"); + ret.append(" }\n"); + ret.append(" }]\n"); + ret.append(" }\n"); + ret.append(" }\n"); + ret.append("});\n"); + ret.append("\n"); return ret; } From 211b8ae212c2fca209d68f4583f6ebbc8644ff31 Mon Sep 17 00:00:00 2001 From: Rahim Kanji Date: Mon, 19 Feb 2024 16:28:06 +0500 Subject: [PATCH 5/6] Fixed memory leak in ProxySQL_Test___Load_MySQL_Whitelist --- lib/ProxySQL_Admin.cpp | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/lib/ProxySQL_Admin.cpp b/lib/ProxySQL_Admin.cpp index f3d521776a..d818d02c28 100644 --- 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::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 From 219fc1b357b2049665359397e3b3c32c99f6d35d Mon Sep 17 00:00:00 2001 From: Rahim Kanji Date: Tue, 20 Feb 2024 10:34:39 +0500 Subject: [PATCH 6/6] Fixed buffer-overflow --- test/tap/tap/utils.cpp | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/test/tap/tap/utils.cpp b/test/tap/tap/utils.cpp index f33a90c198..412686b470 100644 --- a/test/tap/tap/utils.cpp +++ b/test/tap/tap/utils.cpp @@ -423,19 +423,19 @@ int execvp(const string& cmd, const std::vector& argv, string& resu if (err == 0) { // Read from child’s stdout - count = read(PARENT_READ_FD, buffer, sizeof(buffer)); + count = read(PARENT_READ_FD, buffer, sizeof(buffer)-1); while (count > 0) { buffer[count] = 0; result_ += buffer; - count = read(PARENT_READ_FD, buffer, sizeof(buffer)); + count = read(PARENT_READ_FD, buffer, sizeof(buffer)-1); } } else { // Read from child’s stderr - count = read(PARENT_READ_ERR, buffer, sizeof(buffer)); + count = read(PARENT_READ_ERR, buffer, sizeof(buffer)-1); while (count > 0) { buffer[count] = 0; result_ += buffer; - count = read(PARENT_READ_ERR, buffer, sizeof(buffer)); + count = read(PARENT_READ_ERR, buffer, sizeof(buffer)-1); } }