Please upgrade here. These earlier versions are no longer being updated and have security issues.
HackerOne users: Testing against this community violates our program's Terms of Service and will result in your bounty being denied.

Escaping SQL?

Hello,

I'm in the middle of writing a relatively complicated SQL statement which the SQLDriver's build functions just can't handle. I see there's a "Query" function that lets me write plain SQL, but I can't for the life of me figure out how to actually escape text. The "EscapeSQL" method seems to do nothing of the sort. What does a guy need to do to get an escape around here?

Cheers =)
Michael

Comments

  • Yes, it exists:

            $Database = Gdn::Database();
            $Px = $Database->DatabasePrefix;
            $Structure = $Database->Structure();
            $Sql = "your sql here";
            $Structure->Query($Sql);
    

    Don't forget to use "{$Px}TableName" for table names

  • R_JR_J Admin
    edited August 2013

    You could use more native sql even in ->Where() clauses if you use it as such: ->Where('a', 'b', FALSE,FALSE). That way, 'a' and 'b' will not be parsed but only added to the where clause.

  • Thanks @R_J, but I don't actually see how that escapes anything. Lemme give you a more concrete example and explanation about what I mean - we're talking about preventing SQL injection, a critical security issue:

    Say you want to search the Comment table for some text, which is in a variable, $search.

    One might write:

    $DB->Query("SELECT * FROM ".$DB->PrefixTable("Comment")." WHERE Body LIKE '%$search%'");
    

    But say $search contains the text:

    '; DROP TABLE User; SELECT '
    

    That will evaluate to the SQL:

    SELECT * FROM GDN_Comment WHERE Body LIKE '%'; DROP TABLE User; SELECT '%'
    

    ...Which will be executed, causing severe havoc.

    Escaping is something we do to avoid that scenario, replacing, in this case, the single quotes with escaped quotes, so you get:

    SELECT * FROM GDN_Comment WHERE Body LIKE '%\'; DROP TABLE User; SELECT \'%'
    

    ...Which is harmless.

    Normally, we use something like mysqli_real_escape_string for this, but this method requires a DB handle as the first argument.

    Now I could work around that, but my question regards the "Garden"-esque way to accomplish this - which, from what I can see, appears not to exist.

    All there is is the "EscapeSQL" function, which appears to do something utterly inscrutable which is not escaping - if I pass it a piece of text, there's no escaping going on whatsoever, although it does, oddly, add random backticks.

    So, I'm after the equivalent of mysqli_real_escape_string - there must be something like it in there somewhere.

  • You're right, I was talking about something other. I've done a quick search in the code and found QuoteExpression in class.database.php which might do the replacement as you need it

  • Funny is, that it is used nowhere in the code...

  • I think that you should use the named parameters, which are handled by the Gdn_SQLDriver class. If you look at the signature of Gdn_SQLDriver::Query(), you will see that you can pass an array of PDO parameters.

    "Random" usage example, taken from UserModel class:


    By using PDO Parameters, SQL injection should no longer be problem.

    By the way, if the query you need to write is complicated, I would suggest checking if it can be simplified (I'm not implying that you didn't already, it's just that, in my experience, queries can often be simpler that originally expected), or use Database Views. That's what I do in most of my plugins, so that I don't have to jump through too many hoops when building the queries with Vanilla objects.

  • Thanks, guys!

    I think QuoteExpression may be the least worst path forward here..

    The query is simple-ish, but it's including a REGEXP match, and a subquery which is necessitated by some performance issues. It's also built up with a few conditionals, which makes using PDO parameters a little awkward.

  • Can I advise to show the query here or the purpose of the query. We have at least 2 really good DB admins here and some people who would like to help too :-)

    There was an error rendering this rich post.

  • x00x00 MVP
    edited August 2013

    @MichaelTyson said:
    Thanks, guys!

    I think QuoteExpression may be the least worst path forward here..

    The query is simple-ish, but it's including a REGEXP match, and a subquery which is necessitated by some performance issues. It's also built up with a few conditionals, which makes using PDO parameters a little awkward.

    Businessdad gave the best answer.

    Nowadays escaping is done with the database driver functions using a process called "named parameters". The reason is it is knows the databases setup and table schema.

    Anything you do outside of that makes certain assumptions, and can go wrong.

    named parameters are employed automatically if you use the query builder. if you have a good reason not to use the query builder, then you can still use them as per businessdad's example.

     $Sql = "insert {$Px}UserMeta (UserID, Name, Value) values(:UserID, :Name, :Value) on duplicate key update Value = :Value1";
    ....
     Gdn::Database()->Query($Sql, array(':UserID' => $UserID, ':Name' => $Name, ':Value' => $Value, ':Value1' => $Value));
    

    if you have developed wordpress you have probably heard of wpdb::prepare(). This is a better implementation, but like that it takes the query as a whole an insert the params appropriately, in context.

    grep is your friend.

  • @MichaelTyson said:
    The query is simple-ish, but it's including a REGEXP match, and a subquery which is necessitated by some performance issues. It's also built up with a few conditionals, which makes using PDO parameters a little awkward.

    I don't want to sound like a "know it all", but I can't imagine how a sub-query would actually improve performances. Regarding the conditionals, they are definitely easier to handle with the Vanilla's query builder (concatenating SQL "as needed" is rarely a good idea). All of this, obviously, is just a personal opinion. Sorry for going off topic.

  • Cheers for the great info, folks =) As it turns out, my question is now obsolete as it looks like the result won't ever reach production - but I've made it work acceptably on my own site, which is all I really need. Next time, I'll play with named parameters though!

  • Oh, just for academic purposes: I agree with you @businessdad on the dubiousness of subqueries, but oddly enough, it seemed to be the only way to go. I was doing a REGEXP match on Comments.Body and Discussions.Body in the one query and, for reasons I didn't have the time or inclination to investigate further, the query seemed to never finish unless one of them was removed. Doing the search in a subquery appeared to be the only way to make the query work.

    No doubt there's an explanation, but I didn't have the time to go chase it down...

Sign In or Register to comment.