#1
  1. No Profile Picture
    Registered User
    Devshed Newbie (0 - 499 posts)

    Join Date
    Jul 2013
    Posts
    24
    Rep Power
    0

    Going from one php file to another php file using javascript


    Hi all.

    I'm working on my AddAssignment.php page that will assign a builder to a user via a drop down list. When I select the user (i.e. Super in the code below) it will only load the builders that user is NOT already assigned to. It does this by using some javascript and going to another file: getuser.php. I'm wondering how can I make it secure? I'm worried about the line of code:
    PHP Code:
    $q=$_GET["q"]; 
    in getuser.php. I'm reading about SQL injections and I'm freaking out

    Any suggestions would be appreciated! Thank you in advance.

    AddAssignment.php code:
    Code:
    <?php 
    // First we execute our common code to connection to the database and start the session 
    require("common.php"); 
    if(!$_SESSION['user']){ 
        header("Location: index.php"); 
        } 
        
        $smt = $db->prepare('select userid, firstname, lastname From Users'); 
        $smt->execute(); 
        $data = $smt->fetchAll(); 
        
        if(!empty($_POST)) { 
            $query_params = array( 
                ':super' => $_POST['Super'], 
                ':buildercommunity' => $_POST['BuilderCommunity'] 
                ); 
                
                $query = " 
                    INSERT INTO FieldSuperAssignment 
                    ( 
                    BuilderCommunityID, 
                        UserID 
                    ) 
                    VALUES ( 
                        :buildercommunity,
                        :super
                         
                        ) 
                ";  
    try 
            {
            $stmt = $db->prepare($query); 
            $result = $stmt->execute($query_params); 
            } 
            catch(PDOException $ex) 
            { 
                    die("Failed to run query: " . $ex->getMessage()); 
            } 
             header("location: AddAssignment.php");
    } 
    ?>
    <title>Super Administration</title>
    <html xmlns="http://www.w3.org/1999/xhtml"><head>
    <meta http-equiv="content-type" content="text/html; charset=utf-8">
    <link rel="stylesheet" href="style.css" type="text/css">
    <script>
    function showUser(str)
    {
    if (str=="")
    {
    document.getElementById("BuilderCommunity").innerHTML="";
    return;
    } if (window.XMLHttpRequest)
    {// code for IE7+, Firefox, Chrome, Opera, Safari
    xmlhttp=new XMLHttpRequest();
    }
    else
    {// code for IE6, IE5
    xmlhttp=new ActiveXObject("Microsoft.XMLHTTP");
    }
    xmlhttp.onreadystatechange=function()
    {
    if (xmlhttp.readyState==4 && xmlhttp.status==200)
    {
    document.getElementById("BuilderCommunity").innerHTML=xmlhttp.responseText;
    }
    }
    xmlhttp.open("GET","getuser.php?q="+str,true);
    xmlhttp.send();
    }
    </script>
    </head> 
    
    <body>
    <div class="wrapper">
    <div id="logo"></div>
    <form class="form3" action="AddAssignment.php" method="post">
    <div class="formtitle3">Add an Assignment</div>
    <br>
    Field Super:
    <select name="Super" id="Super" onchange="showUser(this.value)"><option selected="selected"> Select Super </option>
    <?php foreach($data as $row) { printf("<option value='%s'>%s %s</option>", $row['userid'], $row['firstname'], $row['lastname']); }?>
    </select>
    <br>
    <br>
    Builder/Community:
    <select name="BuilderCommunity" id="BuilderCommunity"><option selected="selected"> Select Builder/Community </option></select>
    <br>
    <br>
    <div class="buttons" align="center"> <input class="button1" value="Submit" type="submit"> </div>
    </form>
    </div>
    </body></html>
    getuser.php code:
    Code:
    <?php
        // First we execute our common code to connection to the database and start the session 
        require("common.php");
        
        if(!$_SESSION['user']){
            header("Location: index.php"); 
            }
        
    $q=$_GET["q"];
    $smt1 = $db1->prepare('SELECT BuilderCommunityID, Builder, Community FROM BuilderCommunity WHERE BuilderCommunityID NOT IN (SELECT FieldSuperAssignment.BuilderCommunityID FROM FieldSuperAssignment WHERE UserID = '.$q.' )');
    $smt1->execute();
    $data1 = $smt1->fetchAll();
    
    
    foreach ($data1 as $row){
        printf("<option value='%s'>%s --- %s </option>", $row['BuilderCommunityID'], $row['Builder'], $row['Community']);
    }
    
    ?>
    I tried doing this in my getuser.php file but it wasn't displaying anything in the drop down list:

    PHP Code:
    <?php
     
    require("common.php");
        
        if(!
    $_SESSION['user']){
            
    header("Location: index.php"); 
            }
      
           
                
    $smt1 $db1->prepare('SELECT BuilderCommunityID, Builder, Community FROM BuilderCommunity WHERE BuilderCommunityID NOT IN (SELECT FieldSuperAssignment.BuilderCommunityID FROM FieldSuperAssignment WHERE UserID = :q )');

    $smt1->execute(array(':q',$_GET["q"]));
    $data1 $smt1->fetchAll();


    foreach (
    $data1 as $row){
        
    printf("<option value='%s'>%s --- %s </option>"$row['BuilderCommunityID'], $row['Builder'], $row['Community']);
    }

    ?>
  2. #2
  3. No Profile Picture
    Registered User
    Devshed Newbie (0 - 499 posts)

    Join Date
    Jul 2013
    Posts
    24
    Rep Power
    0
    I figured out what I was doing wrong.

    Originally, I was doing this:
    PHP Code:
    $smt1->execute(array(:q,$_GET["q"])); 
    But I changed it to this and it works:
    PHP Code:
    $smt1->execute(array(':q' => $_GET["q"])); 
    Is the code properly secured against injections now (including AddAssignment.php code)?

    PHP Code:
    <?php
     
    require("common.php");
        
        if(!
    $_SESSION['user']){
            
    header("Location: index.php"); 
            }
    $smt1 $db1->prepare('SELECT BuilderCommunityID, Builder, Community FROM BuilderCommunity WHERE BuilderCommunityID NOT IN (SELECT FieldSuperAssignment.BuilderCommunityID FROM FieldSuperAssignment WHERE UserID = :q)');

    $smt1->execute(array(':q' => $_GET["q"]));
    $data1 $smt1->fetchAll();

    foreach (
    $data1 as $row){
    printf("<option value='%s'>%s --- %s </option>"$row['BuilderCommunityID'], $row['Builder'], $row['Community']);
    }



    ?>
  4. #3
  5. --
    Devshed Expert (3500 - 3999 posts)

    Join Date
    Jul 2012
    Posts
    3,959
    Rep Power
    1014
    Hi,

    your code is better than what we usually see, but it's not secure. And there are several other issues.

    The PDO extension you're using has a bug (or "feature"): By default, it uses fake prepared statements, which can be vulnerable to SQL injections. See the link for the details. When you use PDO, you must turn off prepared statements:

    PHP Code:
    $db_options = array(
        
    PDO::ATTR_EMULATE_PREPARES => false                     // important! use actual prepared statements (default: emulate prepared statements)
        
    PDO::ATTR_ERRMODE => PDO::ERRMODE_EXCEPTION           // throw exceptions on errors (default: stay silent)
        
    PDO::ATTR_DEFAULT_FETCH_MODE => PDO::FETCH_ASSOC      // fetch associative arrays (default: mixed arrays)
    );
    $database = new PDO('mysql:host=localhost;dbname=YOURDB;charset=utf8''YOURUSER''YOURPW'$db_options);    // important! specify the character encoding in the DSN string, don't use SET NAMES 
    The next problem is that you have no HTML escaping whatsoever, which can make your code vulnerable to JavaScript injections (aka cross-site scripting). You must escape every value before you can insert it into your HTML document.

    The third issue is that your code leaks internal error messages. Not sure who invented this catch (PDOException $ex) nonsense, but this is very dangerous, because it exposes the exact query, the state of the server and other technical information to the general public. This helps attackers and irritates legitimate users. Apart from that, it's completely useless.

    The fourth issue is that you have no protection against forged requests. You only check the session ID, which means you accept any request coming from a browser with a valid session cookie. This allows an attacker to take actions in the name of another user by controlling their browser (through JavaScript or by making the user click on some button).

    See The 6 worst sins of security for further info.

    Last but not least, you do not stop the script after sending the Location header. This way it keeps running and executes the queries regardless of whether the user is logged in or not. This is another huge security hole. You must put an exit; after the header() call.
    Last edited by Jacques1; August 9th, 2013 at 07:17 PM.
    The 6 worst sins of security ē How to (properly) access a MySQL database with PHP

    Why canít I use certain words like "drop" as part of my Security Question answers?
    There are certain words used by hackers to try to gain access to systems and manipulate data; therefore, the following words are restricted: "select," "delete," "update," "insert," "drop" and "null".
  6. #4
  7. No Profile Picture
    Registered User
    Devshed Newbie (0 - 499 posts)

    Join Date
    Jul 2013
    Posts
    24
    Rep Power
    0
    Thank you for your reply Jacques. It is very helpful as I continue learning.

    (1) I went ahead and added the code to turn off prepared statements as you showed in your code example to my common.php file. and removed PDO::MYSQL_ATTR_INIT_COMMAND => 'SET NAMES utf8' portion from $options Is this the correct way?

    common.php file:
    PHP Code:
    <?php
     $username 
    "myusername"
        
    $password "mypassword"
        
    $host "myhost"
        
    $dbname "mydatabase";
     
    $options = array(  PDO::ATTR_EMULATE_PREPARES => false ,
                        
    PDO::ATTR_ERRMODE => PDO::ERRMODE_EXCEPTION,
                        
    PDO::ATTR_DEFAULT_FETCH_MODE => PDO::FETCH_ASSOC    
        
    ); 

    $db = new PDO("mysql:host={$host};dbname={$dbname};charset=utf8"$username$password$options); 

    $db->setAttribute(PDO::ATTR_ERRMODEPDO::ERRMODE_EXCEPTION);

    $db->setAttribute(PDO::ATTR_DEFAULT_FETCH_MODEPDO::FETCH_ASSOC); 

         function 
    html_escape($raw_input) { 
        return 
    htmlspecialchars($raw_inputENT_QUOTES ENT_HTML401'UTF-8');     


        if(
    function_exists('get_magic_quotes_gpc') && get_magic_quotes_gpc()) 
        { 
            function 
    undo_magic_quotes_gpc(&$array
            { 
                foreach(
    $array as &$value
                { 
                    if(
    is_array($value)) 
                    { 
                        
    undo_magic_quotes_gpc($value); 
                    } 
                    else 
                    { 
                        
    $value stripslashes($value); 
                    } 
                } 
            } 
         
            
    undo_magic_quotes_gpc($_POST); 
            
    undo_magic_quotes_gpc($_GET); 
            
    undo_magic_quotes_gpc($_COOKIE); 
        } 

        
    header('Content-Type: text/html; charset=utf-8'); 

        
    session_start();
    (2) Will reply with this later.

    (3) I have removed the try-catch where I'm only printing an error message. I have read that I shouldn't use try-catch unless I actually want to do something if the query fails, thank you for that.

    (4) To use tokens as your "6 worst sins of security" article states, do I add the tokens portion only to php pages where I use POST or GET? For example, I have a Menu.php file that simply takes the end user to specific .php files (i.e. Data Entry, Register form, logout). So for those files do I continue using :

    PHP Code:
        if(!$_SESSION['user']){
     
    header("Location: index.php");
     exit(); 
    (5) I have added exit; after every instant where I call the Location header
  8. #5
  9. --
    Devshed Expert (3500 - 3999 posts)

    Join Date
    Jul 2012
    Posts
    3,959
    Rep Power
    1014
    Originally Posted by php_newbie05
    (1) I went ahead and added the code to turn off prepared statements as you showed in your code example to my common.php file. and removed PDO::MYSQL_ATTR_INIT_COMMAND => 'SET NAMES utf8' portion from $options Is this the correct way?
    Yes. Now remove all the setAttribute(), because you've already set those options in the constructor (with the $options array).

    By the way, the SET NAMES command is a huge security risk, because it changes the character encoding of the connection without notifying the PDO object. The escaping functions will now assume the wrong character encoding and may no longer recognize the characters they have to escape.

    Luckily, most modern character encodings today are compatible with regard to the "dangerous" characters like ', " etc., so most people get away with this gross mistake. But this is very, very thin ice. In the worst case, you'll have a PDO object without any escaping capabilities.

    Never use SET NAMES.



    Originally Posted by php_newbie05
    (3) I have removed the try-catch where I'm only printing an error message. I have read that I shouldn't use try-catch unless I actually want to do something if the query fails, thank you for that.
    Right. The problem is that die(), echo etc. are dumb: They simply print the message on standard output, regardless of whether the application runs on your local PC for testing or on a live server with 100,000 visitors per second.

    When publishing the code, this forces you to choose between two evils: Either you go through every single file and replace the die()s with an appropriate function call. Or you risk spilling the screens of your users with weird error messages, which is irritating, unprofessional and dangerous (for you).

    If you simply let the exception bubble up, it will automatically send the error message to the right location as specified in the PHP configuration.



    Originally Posted by php_newbie05
    (4) To use tokens as your "6 worst sins of security" article states, do I add the tokens portion only to php pages where I use POST or GET? For example, I have a Menu.php file that simply takes the end user to specific .php files (i.e. Data Entry, Register form, logout). So for those files do I continue using :
    This protection is only relevant for actions. If you simply wanna grant or deny access to a certain page, you don't need a token. But you do need it for every user action (like "add an entry", "change the settings", "log out the user" etc.). To be exact: You need it for every action which requires privileges. You don't have to protect actions anybody can do (think of posting in a guest book or something like that).
    The 6 worst sins of security ē How to (properly) access a MySQL database with PHP

    Why canít I use certain words like "drop" as part of my Security Question answers?
    There are certain words used by hackers to try to gain access to systems and manipulate data; therefore, the following words are restricted: "select," "delete," "update," "insert," "drop" and "null".
  10. #6
  11. No Profile Picture
    Registered User
    Devshed Newbie (0 - 499 posts)

    Join Date
    Jul 2013
    Posts
    24
    Rep Power
    0
    Ok, just got back into town and ready to continue working on the webpage. I have updated AddAssignment.php and common.php to include the htmlescape function you provided in your 6 worst sins of security article. This is how the HTML portion of AddAssignment.php looks like now. Is this correct?

    common.php:
    Code:
         function html_escape($raw_input) { 
        return htmlspecialchars($raw_input, ENT_QUOTES | ENT_HTML401, 'UTF-8');   
    }
    In the above code, I put ENT_HTML401, but how do I know if I'm using HTML 4.01 or HTML5? I've read that if my website doesn't work on Internet Explorer, then it is HTML5. I opened my website on Internet Explorer 8 and some of the CSS doesn't show, and when I click certain buttons it doesn't take me to the file, it just stays in the same page after clicking the button. I also put <!DOCTYPE html> at the top of my html code. Does this mean it is in HTML5?

    AddAssignment.php:
    Code:
    <?php 
    PHP code 
    ?>
    <!DOCTYPE html>
    <head>
    <meta http-equiv="content-type" content="text/html; charset=utf-8">
    <link rel="stylesheet" href="style.css" type="text/css">
    </head> 
    
    <body>
    <div class="wrapper">
    <div id="logo"></div>
    <form class="form3" action="AddAssignment.php" method="post">
    <div class="formtitle3">Add an Assignment</div>
    <br>
    Field Super:
    <select name="Super" id="Super"><option selected="selected"> Select Super </option>
    <?php foreach($data as $row) { printf("<option value='%s'>%s %s</option>", html_escape($row['userid']), html_escape($row['firstname']), html_escape($row['lastname'])); }?>
    </select>
    <br>
    <br>
    Builder/Community:
    <select name="BuilderCommunity" id="BuilderCommunity"><option selected="selected"> Select Builder/Community </option>
    <?php foreach($data1 as $row) { printf("<option value='%s'>%s --- %s </option>", html_escape($row['BuilderCommunityID']), html_escape($row['Builder']), html_escape($row['Community'])); }?>
    </select>
    <br>
    <br>
    <div class="buttons" align="center"> <input class="button1" value="Submit" type="submit"/>  <input class="button1" type="submit" formaction="Menu.php" style="width:90" value="Menu" /></div>
    </form>
    </div>
    </body></html>
    Thanks again Jacques!
  12. #7
  13. --
    Devshed Expert (3500 - 3999 posts)

    Join Date
    Jul 2012
    Posts
    3,959
    Rep Power
    1014
    Originally Posted by php_newbie05
    This is how the HTML portion of AddAssignment.php looks like now. Is this correct?
    Yes.



    Originally Posted by php_newbie05
    In the above code, I put ENT_HTML401, but how do I know if I'm using HTML 4.01 or HTML5?
    Contrary to popular belief, the browser doesn't care about HTML 4.01, HTML5 or whatever. It knows HTML and supports particular features of it.

    The problem of HTML5 is that it's relatively new and still a work in progress. Some modern browsers support the HTML5 features pretty well, but others don't.

    So it basically comes down to two simple questions: How important is it for you to have the newest features? And what's your target audience? If you need the latest and greatest and if your users have the same attitude, you can go with the HTML5 features. If you need to support old browsers (like Internet Explorer 8), restrict yourself to the HTML 4.01 features.

    The doctype is always

    Code:
    <!DOCTYPE html>
    The HTML version in the htmlspecialchars() isn't important and only changes the way a single quote is encoded. I use it for documentation purposes.



    Originally Posted by php_newbie05
    I've read that if my website doesn't work on Internet Explorer, then it is HTML5.
    No, that's nonsense.
    The 6 worst sins of security ē How to (properly) access a MySQL database with PHP

    Why canít I use certain words like "drop" as part of my Security Question answers?
    There are certain words used by hackers to try to gain access to systems and manipulate data; therefore, the following words are restricted: "select," "delete," "update," "insert," "drop" and "null".
  14. #8
  15. No Profile Picture
    Registered User
    Devshed Newbie (0 - 499 posts)

    Join Date
    Jul 2013
    Posts
    24
    Rep Power
    0
    Great thank you for all of your help Jacques!

    I will continue to use the security best practices you have in your article and that you have help me with here. Thanks again.
  16. #9
  17. --
    Devshed Expert (3500 - 3999 posts)

    Join Date
    Jul 2012
    Posts
    3,959
    Rep Power
    1014
    Originally Posted by php_newbie05
    I will continue to use the security best practices you have in your article and that you have help me with here.
    Cool. I wish you good luck with your projects, and if you have any question, just ask.
    The 6 worst sins of security ē How to (properly) access a MySQL database with PHP

    Why canít I use certain words like "drop" as part of my Security Question answers?
    There are certain words used by hackers to try to gain access to systems and manipulate data; therefore, the following words are restricted: "select," "delete," "update," "insert," "drop" and "null".

IMN logo majestic logo threadwatch logo seochat tools logo