Refugnic Eternium
Refugnic Eternium

Reputation: 4291

Combine mysql_real_escape string with sprintf

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

Answers (1)

Refugnic Eternium
Refugnic Eternium

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:

  1. Determine, if there's any formatting to be done at all. (Not much use wasting cycles on expanding and stuff, if there aren't any arguments in it)
  2. Determine the required length (including 0) and error checking.
  3. Reserve sufficient room for one full expansion, set the marker and expand.
  4. Check, if there are any 'strings to be escaped' expected (marked by '')
  5. Check, if the control character 1 is somewhere in the expansion. If so, you'll know this is an attempted attack and can deny it right there.
  6. Replace every occurrence of the apostrophe with the control character of your choice. (The one, which must not be passed as parameter)
  7. Expand the query again with the altered format string.
  8. Look for the control characters as delimiters for the string and copy the whole thing into the final buffer, which is then passed to mysql
  9. Free the dynamic memories as they expire.

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

Related Questions