Full Checks Before Adding To Database – Sql Injections, Html Tags – The Lot

Discussion in 'PHP' started by johnsmith153, May 10, 2008.

  1. #1
    This is more of a solution. I am asking for confirmation that this is suitable and so I can go ahead and use on my new site.

    A few answers would be great:

    (1) Is below suitable to ensure very good security on my site? Do I need anything else?
    (2) I am using <meta http-equiv="Content-Type" content="text/html;charset=utf-8" /> for ALL pages. There is a bit with htmlentities where I have selected 'UTF-8' – Is this correct?
    (3) Will I be covered by people trying to enter the following?
    -- (I have heard putting this can signal to end a command)
    ;
    =
    \
    +
    Entering char()
    Using &# to display html characters
    ASCII / Binary or something?

    I cant / wont use:
    mysql_real_escape_string (I am using a MySQL database, but through a web services API – so doesn't work)
    stripslashes / addslashes / magic_quotes (deprecated php 6)

    Imagine for this example a user is submitting a message to a message board. And for example's sake, a form posts the message, name of user and age of user (assume no log in – they just type in their name, age and message.)

    I will always use fixed column names and never dynamically create them (i.e I will never use ORDER BY '$columnname' ) – however I will often use dynamic statements (i.e I WILL use - Customer LIKE '$customername')

    PREPARE DATA TO ADD TO DATABASE

    <?php

    global $error_chk;
    $error_chk = 0;
    function checkdata($value, $valtype, $length)
    {

    global $error_chk;

    $value = trim($value);
    $value = htmlentities($value, ENT_NOQUOTES, 'UTF-8');
    $value = str_replace("#", "\#", $value);
    $value = str_replace("%", "\%", $value);
    $value = str_replace("_", "\_", $value);
    $value = str_replace("\x00", "\\x00", $value);
    $value = str_replace("\n", "\\n", $value);
    $value = str_replace("\r", "\\r", $value);
    $value = str_replace("\\", "\\\\", $value);
    $value = str_replace("'", "\'", $value);
    $value = str_replace("\"", "\\\"", $value);
    $value = str_replace("\x1a", "\\\x1a", $value);

    if($valtype=="st"){if(!is_string($value)){$error_chk = 1;$value="";}}
    if($valtype=="ar"){if(!is_array($value)){$error_chk = 1;$value="";}}
    if($valtype=="in"){if(!is_numeric($value))
    {$error_chk = 1;$value="";}else{intval($value);}}
    if($valtype=="fl"){if(!is_numeric($value))
    {$error_chk = 1;$value="";}else{floatval($value);}}

    if($valtype=="st"){if(strlen($value)>$length){$error_chk=2;$value="";}}
    if($valtype=="in"){$value>$length){$error_chk=2;$value="";}}

    }

    if($error_chk!=0)
    {
    echo "DONT ADD TO DB";
    }

    //Now I can do this: (I think)
    //st=string in=integer etc.
    $messagetoadd = checkdata($_POST['message'], 'st', 250);
    $name = checkdata($_POST['name'], 'st', 16);
    $age = checkdata($_POST['age'], 'in', 110);

    ?>


    PREPARE DATA TO SHOW ON SCREEN TO USER

    Reverse the above
    i.e. use: html_entity_decode($value, ENT_NOQUOTES, 'UTF-8');


    Thanks for any help.
     
    johnsmith153, May 10, 2008 IP
  2. johnsmith153

    johnsmith153 Peon

    Messages:
    17
    Likes Received:
    0
    Best Answers:
    0
    Trophy Points:
    0
    #2
    If that is too long to read, please just tell me if this is good enough to validate data before database insert.

    <?php

    global $error_chk;
    $error_chk = 0;
    function checkdata($value, $valtype, $length)
    {

    global $error_chk;

    $value = trim($value);
    $value = htmlentities($value, ENT_NOQUOTES, 'UTF-8');
    $value = str_replace("#", "\#", $value);
    $value = str_replace("%", "\%", $value);
    $value = str_replace("_", "\_", $value);
    $value = str_replace("\x00", "\\x00", $value);
    $value = str_replace("\n", "\\n", $value);
    $value = str_replace("\r", "\\r", $value);
    $value = str_replace("\\", "\\\\", $value);
    $value = str_replace("'", "\'", $value);
    $value = str_replace("\"", "\\\"", $value);
    $value = str_replace("\x1a", "\\\x1a", $value);

    if($valtype=="st"){if(!is_string($value)){$error_chk = 1;$value="";}}
    if($valtype=="ar"){if(!is_array($value)){$error_chk = 1;$value="";}}
    if($valtype=="in"){if(!is_numeric($value))
    {$error_chk = 1;$value="";}else{intval($value);}}
    if($valtype=="fl"){if(!is_numeric($value))
    {$error_chk = 1;$value="";}else{floatval($value);}}

    if($valtype=="st"){if(strlen($value)>$length){$error_chk=2;$value="";}}
    if($valtype=="in"){$value>$length){$error_chk=2;$value="";}}

    }

    if($error_chk!=0)
    {
    echo "DONT ADD TO DB";
    }

    //Now I can do this: (I think)
    //st=string in=integer etc.
    $messagetoadd = checkdata($_POST['message'], 'st', 250);
    $name = checkdata($_POST['name'], 'st', 16);
    $age = checkdata($_POST['age'], 'in', 110);

    ?>
     
    johnsmith153, May 10, 2008 IP
  3. Nalltaroh

    Nalltaroh Peon

    Messages:
    54
    Likes Received:
    4
    Best Answers:
    0
    Trophy Points:
    0
    #3
    Is really good! Congratulations.
     
    Nalltaroh, May 11, 2008 IP
  4. lanmonkey

    lanmonkey Active Member

    Messages:
    549
    Likes Received:
    9
    Best Answers:
    0
    Trophy Points:
    60
    #4
    You are doing way too much. First of all the only thing you really must do BEFORE you insert something to the database is to do a mysql_real_escape_string. That will pretty much cover you for sql injection.

    Displaying the contents of the database in a browser is another matter, but again all you need to do is use htmlentities like you have done above. you could use striptags or something like that but I just use htmlentities on its own because then if someone does try an XSS atack it will allow me to see the entire code they posted without it being executed.

    Dont use html_entity_decode before showing the info on your screen, this will undo the good work that htmlentities did.

    you should use htmlentities on all user input before it is sent to the browser, else you will get XSS atacks.

    Where you are using intval and is_string, is_array etc.. this is pointless when sending data to the DB though you always SHOULD do this when processing data taken from $_POST or $_GET eg:

    $number = intval($_POST['number']);

    that way people can't send the wrong type of data into your script.

    For real in depth info I can recommend the book “Pro PHP Security” by snyder.
     
    lanmonkey, May 12, 2008 IP
  5. johnsmith153

    johnsmith153 Peon

    Messages:
    17
    Likes Received:
    0
    Best Answers:
    0
    Trophy Points:
    0
    #5
    Ianmonkey,

    Read my post. I can't use mysql_real_escsape_string.

    I AM using htmlentities.

    Striptags when displaying to user wont work, as you have already used htmlentities on things like <> - there will be no tags to strip.

    You advised I use $number = intval($_POST['number']);

    I have used $age = checkdata($_POST['age'], 'in', 110); (which is the same as yours but better.)

    Read my script, I DO apply the is_string etc. to the $_POST value - and then send to db.

    I dont need to read a book. Many other people, this and two other forums have said my script is good.

    This message is not to have a go at you, but so others reading this who want to know how to do it will know - my way is a good way - then, they wont waste time trying to change it. - Although if you CAN, do use mysql_real_escape_string instead of:

    $value = str_replace("\x00", "\\x00", $value);
    $value = str_replace("\n", "\\n", $value);
    $value = str_replace("\r", "\\r", $value);
    $value = str_replace("\\", "\\\\", $value);
    $value = str_replace("'", "\'", $value);
    $value = str_replace("\"", "\\\"", $value);
    $value = str_replace("\x1a", "\\\x1a", $value);
     
    johnsmith153, May 12, 2008 IP
  6. lanmonkey

    lanmonkey Active Member

    Messages:
    549
    Likes Received:
    9
    Best Answers:
    0
    Trophy Points:
    60
    #6
    Fair enough, but I beg to differ. If your posts are to help others then by telling them to use htmlentities before the database (pointless, infact it wastes space) and html_entity_decode before showing on user output in the browser is as backwards as it is dangerous.

    Well, yes there will be because your recreating the dangerous <script> tags with html_entity_decode just before you send to the browser!

    Anyway. Its up to you what you do, but I wouldn’t do it.
     
    lanmonkey, May 12, 2008 IP
  7. Altari

    Altari Peon

    Messages:
    188
    Likes Received:
    4
    Best Answers:
    0
    Trophy Points:
    0
    #7
    What are you trying to protect? For forum posts, you're probably OK. If you're securing sensitive data, you're very vulnerable.
     
    Altari, May 13, 2008 IP
  8. johnsmith153

    johnsmith153 Peon

    Messages:
    17
    Likes Received:
    0
    Best Answers:
    0
    Trophy Points:
    0
    #8
    Altari,

    Maybe you could explain a little about how I would be very vulnerable?

    Thanks.
     
    johnsmith153, May 13, 2008 IP
  9. Altari

    Altari Peon

    Messages:
    188
    Likes Received:
    4
    Best Answers:
    0
    Trophy Points:
    0
    #9
    It doesn't appear that you're protecting again MySQL functions. You'd be better off investing the time and energy in bind variables than in filters.

    That being said, for some data there is a "good enough" as far as security, so it all comes down to what you're protecting.
     
    Altari, May 13, 2008 IP