Skip to content

Commit 9b24fdf

Browse files
committed
Prevent crash COM_STMT_EXECUTE and invalid stmt_id
If an invalid stmt_id is passed to COM_STMT_EXECUTE , ProxySQL used to crash due to an assert(). It seems that some buggy clients execute COM_STMT_EXECUTE with an invalid stmt_id after an auto-reconnect. This commit returns an error to the client. Closes #3371 Closes #3808 Closes #4474
1 parent 3187ee6 commit 9b24fdf

File tree

2 files changed

+98
-5
lines changed

2 files changed

+98
-5
lines changed

lib/MySQL_Session.cpp

+9-5
Original file line numberDiff line numberDiff line change
@@ -3790,10 +3790,13 @@ void MySQL_Session::handler___status_WAITING_CLIENT_DATA___STATE_SLEEP___MYSQL_C
37903790
CurrentQuery.stmt_client_id=client_stmt_id;
37913791
stmt_global_id=client_myds->myconn->local_stmts->find_global_stmt_id_from_client(client_stmt_id);
37923792
if (stmt_global_id == 0) {
3793-
// FIXME: add error handling
3794-
// LCOV_EXCL_START
3795-
assert(0);
3796-
// LCOV_EXCL_STOP
3793+
l_free(pkt.size,pkt.ptr);
3794+
client_myds->setDSS_STATE_QUERY_SENT_NET();
3795+
string err_msg = "Unknown prepared statement handler (" + to_string(client_stmt_id) + ") given to mysql_stmt_precheck";
3796+
client_myds->myprot.generate_pkt_ERR(true,NULL,NULL,1,1243,(char *)"HY000", err_msg.c_str());
3797+
client_myds->DSS=STATE_SLEEP;
3798+
status=WAITING_CLIENT_DATA;
3799+
return;
37973800
}
37983801
CurrentQuery.stmt_global_id=stmt_global_id;
37993802
// now we get the statement information
@@ -3803,7 +3806,8 @@ void MySQL_Session::handler___status_WAITING_CLIENT_DATA___STATE_SLEEP___MYSQL_C
38033806
// we couldn't find it
38043807
l_free(pkt.size,pkt.ptr);
38053808
client_myds->setDSS_STATE_QUERY_SENT_NET();
3806-
client_myds->myprot.generate_pkt_ERR(true,NULL,NULL,1,1045,(char *)"28000",(char *)"Prepared statement doesn't exist", true);
3809+
string err_msg = "Unknown prepared statement handler (" + to_string(client_stmt_id) + ") given to mysql_stmt_precheck";
3810+
client_myds->myprot.generate_pkt_ERR(true,NULL,NULL,1,1243,(char *)"HY000", err_msg.c_str());
38073811
client_myds->DSS=STATE_SLEEP;
38083812
status=WAITING_CLIENT_DATA;
38093813
return;
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,89 @@
1+
/**
2+
* @file reg_test_3371_prepared_statement_crash-t.cpp
3+
* @brief Tries to execute prepared statements with a not existing stmt_id.
4+
* This used to crash ProxySQL , so this tap test verifies that ProxySQL
5+
* doesn't crash
6+
*/
7+
8+
9+
#include <string>
10+
#include <stdio.h>
11+
#include <cstring>
12+
#include <unistd.h>
13+
14+
#include "mysql.h"
15+
16+
#include "tap.h"
17+
#include "command_line.h"
18+
#include "utils.h"
19+
20+
using std::string;
21+
22+
const int NUM_LOOPS = 100; ///< Number of loops for statement execution.
23+
24+
int main(int argc, char** argv) {
25+
CommandLine cl;
26+
27+
// Checking for required environmental variables
28+
if (cl.getEnv()) {
29+
diag("Failed to get the required environmental variables.");
30+
return -1;
31+
}
32+
33+
plan(1+NUM_LOOPS*2); // Plan for testing purposes
34+
35+
MYSQL* mysql = mysql_init(NULL); ///< MySQL connection object
36+
if (!mysql) {
37+
fprintf(stderr, "File %s, line %d, Error: %s\n", __FILE__, __LINE__, mysql_error(mysql));
38+
return exit_status();
39+
}
40+
41+
// Connecting to ProxySQL
42+
diag("Connecting to '%s@%s:%d'", cl.mysql_username, cl.mysql_host, cl.port);
43+
if (!mysql_real_connect(mysql, cl.mysql_host, cl.mysql_username, cl.mysql_password, NULL, cl.port, NULL, 0)) {
44+
fprintf(stderr, "File %s, line %d, Error: %s\n", __FILE__, __LINE__, mysql_error(mysql));
45+
return exit_status();
46+
}
47+
48+
// Initialize and prepare all the statements
49+
MYSQL_STMT* stmt = mysql_stmt_init(mysql);
50+
if (!stmt) {
51+
fprintf(stderr, "mysql_stmt_init(), out of memory\n");
52+
return exit_status();
53+
}
54+
55+
std::string select_query = "SELECT 1";
56+
diag("select_query: %s", select_query.c_str());
57+
if (mysql_stmt_prepare(stmt, select_query.c_str(), strlen(select_query.c_str()))) {
58+
fprintf(stderr, "mysql_stmt_prepare at line %d failed: %s\n", __LINE__ , mysql_error(mysql));
59+
mysql_close(mysql);
60+
mysql_library_end();
61+
return exit_status();
62+
}
63+
64+
diag("Increasing stmt_id by 1, so that mysql_stmt_execute() must fail");
65+
int rc = 0;
66+
for (int i = 0; i < NUM_LOOPS ; i++) {
67+
stmt->stmt_id += 1;
68+
rc = mysql_stmt_execute(stmt);
69+
ok (rc , "mysql_stmt_execute() must fail");
70+
if (rc) {
71+
unsigned int psrc = mysql_stmt_errno(stmt);
72+
ok( psrc == 1243 , "mysql_stmt_execute at line %d failed: %d , %s", __LINE__ , psrc , mysql_stmt_error(stmt));
73+
}
74+
}
75+
76+
diag("Decreasing stmt_id by 1, so that mysql_stmt_execute() must succeed");
77+
stmt->stmt_id -= NUM_LOOPS;
78+
rc = mysql_stmt_execute(stmt);
79+
ok (rc == 0 , "mysql_stmt_execute() succeeded");
80+
if (rc) {
81+
fprintf(stderr, "mysql_stmt_execute at line %d failed: %d , %s\n", __LINE__ , rc , mysql_stmt_error(stmt));
82+
mysql_close(mysql);
83+
mysql_library_end();
84+
return exit_status();
85+
}
86+
mysql_close(mysql);
87+
88+
return exit_status();
89+
}

0 commit comments

Comments
 (0)