Securing User Input

Discussion in 'PHP' started by Silver89, Aug 9, 2010.

  1. #1
    Will the following remove any unsafe code from a user input? There will be more tidying up to the variable names etc before it's used on a live site.

    
    	$body = strip_tags($_POST[body], '<p><a><b><strong><i><u><img><strike><sub><sup><ol><ul><li><blockquote><object><param><embed><hr><br><table><tbody><tr><td><h1><h2><h3><h4><h5><span>');
    
    	$body = str_replace('<?php', '', $body);
    	$body = str_replace('<?', '', $body);
    	$body = str_replace('?>', '', $body);
    	$body = preg_replace('#<(script|style)[^>]*>.+?</\1>#i', '', $body);
    
    
    PHP:
    It should strip everything apart from the allowed tags, have I missed anything out?

    Thanks
     
    Silver89, Aug 9, 2010 IP
  2. Rainulf

    Rainulf Active Member

    Messages:
    373
    Likes Received:
    12
    Best Answers:
    0
    Trophy Points:
    85
    #2
    Good job. But I'm not sure if you should allow <object>, <param> or <embed> - people can do funny things with it. You may also wanna consider mysqli_real_escape_string( ). ;)

    I think this is pointless:
    
        $body = str_replace('<?php', '', $body);
        $body = str_replace('<?', '', $body);
        $body = str_replace('?>', '', $body);
    
    PHP:
     
    Rainulf, Aug 9, 2010 IP
  3. danx10

    danx10 Peon

    Messages:
    1,179
    Likes Received:
    44
    Best Answers:
    2
    Trophy Points:
    0
    #3
    use htmlspecialchars() when displaying user input...and mysql_real_escape_string when saving user input to the db.

    furthermore I don't see why your allowing certain tags, its best practive to remove all
     
    danx10, Aug 9, 2010 IP
  4. HuggyEssex

    HuggyEssex Member

    Messages:
    297
    Likes Received:
    4
    Best Answers:
    2
    Trophy Points:
    45
    #4
    I would firstly make sure if gpc_get_magic_quotes is on using a simple if check, if it's not on add slashes to the input, if it's on just use strip_tags. Remember you can't make something impossible to actually hack you can only make it very difficult.

    Glen
     
    HuggyEssex, Aug 10, 2010 IP
  5. Silver89

    Silver89 Notable Member

    Messages:
    2,243
    Likes Received:
    72
    Best Answers:
    0
    Trophy Points:
    205
    #5
    Won't that prevent php from being stored in the database or does mysqli_real_escape_string() do that?

    Well I want to allow users to format their posts with some html
     
    Silver89, Aug 10, 2010 IP
  6. bigmax

    bigmax Peon

    Messages:
    33
    Likes Received:
    0
    Best Answers:
    0
    Trophy Points:
    0
    #6
    
      foreach ($_POST as $key => $value) { 
        $_POST[$key] = mysql_real_escape_string($value); 
      } 
      foreach ($_GET as $key => $value) { 
        $_GET[$key] = mysql_real_escape_string($value); 
      }
    
    Code (markup):
    placed at the beginning of your processing files is much more prosaic, assuming you work with databases then hehe...
     
    Last edited: Aug 10, 2010
    bigmax, Aug 10, 2010 IP
  7. Rainulf

    Rainulf Active Member

    Messages:
    373
    Likes Received:
    12
    Best Answers:
    0
    Trophy Points:
    85
    #7
    You need to look up what those function do. str_replace( ) is pointless because strip_tags( ) already does the job, it strips PHP tags. mysqli_real_escape_string( ) prevents SQL injections. Just make sure you escape $body, not the whole SQL statement. Or else your query isn't gonna run.

    Here:
    
    $body = strip_tags($_POST['body'], '<p><a><b><strong><i><u><img><strike><sub><sup><ol><ul><li><blockquote><hr><br><table><tbody><tr><td><h1><h2><h3><h4><h5><span>');
    $body = preg_replace('#<(script|style)[^>]*>.+?</\1>#i', '', $body);
    $body = $mysqli->real_escape_string($body); // or procedural mysqli_real_escape_string($body, $connection); 
    
    PHP:
     
    Last edited: Aug 10, 2010
    Rainulf, Aug 10, 2010 IP
  8. nico_swd

    nico_swd Prominent Member

    Messages:
    4,153
    Likes Received:
    344
    Best Answers:
    18
    Trophy Points:
    375
    #8
    Not secure, considering I can use JavaScript in attributes of allowed tags.

    <img src="1" onerror="alert('Hax0red');" />
    HTML:
    If you really want to allow HTML, I suggest HTMLPurifier or just BB code.
     
    nico_swd, Aug 11, 2010 IP
    Silver89 likes this.
  9. Silver89

    Silver89 Notable Member

    Messages:
    2,243
    Likes Received:
    72
    Best Answers:
    0
    Trophy Points:
    205
    #9

    Would this fix that issue?

    
    $body = preg_replace('`on[^=]+=".*?"([ >])`', '$1', $body);
    
    PHP:
     
    Last edited: Aug 11, 2010
    Silver89, Aug 11, 2010 IP
  10. Silver89

    Silver89 Notable Member

    Messages:
    2,243
    Likes Received:
    72
    Best Answers:
    0
    Trophy Points:
    205
    #10
    I decided to go with HTMLPurifier, does the job perfectly with the following:

    
    	$dirty_html = strip_tags($_POST[body], '<p><a><b><strong><i><u><img><strike><sub><sup><ol><ul><li><blockquote><hr><br><table><tbody><tr><td><h1><h2><h3><h4><h5><span>');
    
        require_once 'htmlpurifier/HTMLPurifier.standalone.php';
        
        $purifier = new HTMLPurifier();
        $clean_html = $purifier->purify($dirty_html);
    
    Code (markup):
     
    Silver89, Aug 11, 2010 IP
  11. nico_swd

    nico_swd Prominent Member

    Messages:
    4,153
    Likes Received:
    344
    Best Answers:
    18
    Trophy Points:
    375
    #11
    No. :)

    
    <img src=1 onerror=alert('hax0red'); />
    
    <img src=1 onerror='alert("hax0red");' />
    
    Code (markup):
    Looks good!
     
    nico_swd, Aug 11, 2010 IP