Reputation: 4291
Maybe I'm just too stupid to search again.
Anyway, here's the situation.
To prevent SQL-Injections, I need to use mysql_real_escape_string
, however this function is awfully clunky and requires a 'lot' of extra code. I'd like to keep the function under the wraps of essentially the sprintf
-function.
The idea: Whenever sprintf
encounters a %s
, it would run mysql_real_escape_string
on the corresponding va_arg and then add it to the target string.
Example:
doQuery("SELECT * FROM `table` WHERE name LIKE '%%s%%';", input);
Assuming input
is a string like Tom's diner
, the complete query would look like:
SELECT * FROM `table` WHERE name LIKE '%Tom\'s diner%';
I've found a fairly elegant way to achieve what I want, however there's a security risk connected with it and I'm wondering if there isn't a better way.
Here's what I'm trying:
void doQuery(const char *Format, ...) {
char sQuery[1024], tQuery[1024], *pQuery = sQuery, *pTemp = tQuery;
va_list val;
strcpy(sQuery, Format);
while((pQuery = strchr(pQuery, '\'')) != NULL) *pQuery = 1;
va_start(val, Format);
vsprintf(tQuery, sQuery, val);
va_end(val);
pQuery = sQuery;
do {
if(*pTemp == 1) {
char *pSearch = strchr(pTemp, 1);
if(!pSearch) return; //Error, missing second placeholder
else {
*pQuery++ = '\'';
mysql_real_escape_string(sql, pQuery, pTemp, pSearch - pTemp);
pQuery += strlen(pQuery);
*pQuery++ = '\'';
pTemp = pSearch;
}
} else *pQuery++ = *pTemp;
} while(*pTemp++);
//Execute query, return result, etc.
}
This function was written from memory, I'm not 100 % sure about it's correctness, but I think you get the idea. Anyway, the obvious security risk rests with the placeholder 1. If an attacker got the idea of putting said 1 (numeric value, not the character '1') into the input string, he'd automatically have an attack point, i.e. a non-escaped apostrophe.
Now, does anyone have any idea, how I could fix this problem and still get the behavior I want, preferably without allocating an extra buffer for each and every string I want to send to the database? I'd also like to avoid overriding the entire sprintf-function, if somehow possible.
Thank you very much.
Upvotes: 2
Views: 404
Reputation: 4291
After pondering the problem for a little longer, I believe to have found a rather simple answer, which will serve the purpose well.
I simply need to count the occurrences of the apostrophes I'm replacing with the placeholder and then, while parsing the formatted string, count backwards. If I find the placeholder more often than I counted while the first pass, I'll know that one of the arguments contains an illegal character and is therefor invalid and shall not be passed to the database.
Edit: WAY late, but I think now (as I stumbled over the SAME problem once more) I found a good way. Clunky, but workable.
bool SQL::vQuery(const char *Format, va_list val) {
bool Ret = true, bExpanded = false;
if(strchr(Format, '%') != NULL) { //Is there any expanding to be done here?
int32_t ReqLen = vsnprintf(NULL, 0, Format, val) + 1; //Determine the required buffer length.
if(ReqLen < 2) Ret = false; //Lengthquery successful?
else {
char *Exp = new char[ReqLen]; //Evaluation requires a sufficiently large buffer.
bExpanded = true; //Tell the footer of this function to free the query buffer.
vsprintf(Exp, Format, val); //Expand the string into the first buffer.
if(strchr(Format, '\'') == NULL) Format = Exp; //No apostrophes found in the format(!) string? No escaping necessary.
else if(strchr(Exp, 1)) Ret = false; //Illegal character detected. Abort.
else {
char *pExp = Exp,
*Query = new char[ReqLen * 2], //Reserves (far more than) enough space for escaping.
*pQuery = Query;
strcpy(Query, Format); //Copy the format string to the (modifiable) Query buffer.
while((pQuery = strchr(pQuery, '\'')) != NULL)
*pQuery = 1; //Replace the character with the control character.
vsprintf(Exp, Query, val); //Expand the whole thing AGAIN, this time with the substitutions.
pQuery = Query; //And rewind the pointer.
while(char *pEnd = strchr(pExp, 1)) { //Look for the text-delimiter.
*pEnd = 0; //Terminate the string at this point.
strcpy(pQuery, pExp); //Copy the unmodified string to the final buffer.
pQuery += pEnd - pExp; //And advance the pointer to the new end.
pExp = ++pEnd; //Beginning of the 'To be escaped' string.
if((pEnd = strchr(pExp, 1)) != NULL) { //And what about the end?
*pEnd = 0; //Terminate the string at this point.
*pQuery++ = '\'';
pQuery += mysql_real_escape_string(pSQL, pQuery, pExp, pEnd - pExp);
*pQuery++ = '\'';
pExp = ++pEnd; //End of the 'To be escaped' string.
} else Ret = false; //Malformed query string.
}
strcpy(pQuery, pExp); //No more '? Just copy the rest.
Format = Query; //And please use the Query-Buffer instead of the raw Format.
delete[] Exp; //Get rid of the expansion buffer.
}
}
}
if(Ret) {
if(result) mysql_free_result(result); //Gibt ein ggf. bereits vorhandenes Ergebnis wieder frei.
Ret = mysql_query(pSQL, Format, result);
columns = (result) ? mysql_num_fields(result) : 0;
row = NULL;
}
if(bExpanded) delete[] Format; //Query completed -> Dispose of buffer.
return Ret;
}
What this monstrosity does is the following steps:
I think I'm finally satisfied with this solution...and it only took me 1.5 years to figure it out. :D
I hope this helps someone else as well.
Upvotes: 1