patterncppMinor
My method of escaping MySQL strings in C++
Viewed 0 times
mysqlstringsescapingmethod
Problem
This is how I'm currently escaping MySQL strings in C++:
It seems to be doing the job, but I'm curious if I can get a review of my
std::string escapeSQL(const char* dataIn)
{
if (!MySQL_Database_Connection__global->host)
int a = NULL;
if (dataIn)
{
std::size_t dataInLen = strlen(dataIn);
std::string to(((dataInLen * 2) + 1), '\0');
mysql_real_escape_string(MySQL_Database_Connection__global, &to[0], dataIn, dataInLen);
return (to);
}
else
{
return ("");
}
}
char query[1024] = {'\0'};
snprintf(query, sizeof(query), "SELECT * FROM database.table WHERE column = '%s';", escapeSQL(userTypedArgument).c_str());It seems to be doing the job, but I'm curious if I can get a review of my
escapeSQL() function for any pitfalls I may have overlooked.Solution
if (!MySQL_Database_Connection__global->host)
int a = NULL;What is the purpose of this if statement? It only contains one unused declaration.
if (dataIn) ... return ("");I try to avoid adding defensive behavior like this. Returning empty string for NULL argument seems like a good idea and might even save your program from crashing. But, NULL argument indicates your program is already in invalid state and it's unlikely to function properly. Assertion or exception with clear indication what's wrong would save you a lot of debugging.
std::size_t dataInLen = strlen(dataIn);
escapeSQL(userTypedArgument).c_str()Consider using just std::string or just C strings. Or at least minimize those awkward conversions back and forth. They can only lead to bugs like this one:
mysql_real_escape_string(MySQL_Database_Connection__global, &to[0], dataIn, dataInLen);You are creating null-terminated C string inside
std::string. std::string ignores null characters. to.length() will always return dataInLen*2+1. Any method other than c_str() will result in unexpected behavior.char query[1024] = {'\0'};
snprintf(query, sizeof(query), "SELECT * FROM database.table WHERE column = '%s';", escapeSQL(userTypedArgument).c_str());This is exploitable. You should check if the query exceeds 1023 characters and fail in a safe way (or just use
std::string). Currently user can remove part of your query by entering a long string. For example:"DELETE * FROM table WHERE name='long string ...' AND owner=currentUser"
^ 1024th characterCode Snippets
if (!MySQL_Database_Connection__global->host)
int a = NULL;if (dataIn) ... return ("");std::size_t dataInLen = strlen(dataIn);
escapeSQL(userTypedArgument).c_str()mysql_real_escape_string(MySQL_Database_Connection__global, &to[0], dataIn, dataInLen);char query[1024] = {'\0'};
snprintf(query, sizeof(query), "SELECT * FROM database.table WHERE column = '%s';", escapeSQL(userTypedArgument).c_str());Context
StackExchange Code Review Q#71932, answer score: 4
Revisions (0)
No revisions yet.