sql injections in forms

Discussion in 'PHP' started by DBallerz, Oct 31, 2007.

0
  1. #1
    Is there anyway to improve/change this form to better handle sql injections ?
    
<html>
<body>
<form action="<?php $_SERVER[PHP_SELF]; ?>" method="post"><br />
Name: <input type="text" name="name" /><br />
Age: <input type="text" name="age" /><br />
Description: <textarea name="description" rows="2" cols="20"></textarea>
<input name="submit" type="submit" />
</form>
</body>
</html>

<?php
if (isset($_POST['submit'])) { //check if the form was submitted

if (!empty($_POST['name']) && !empty($_POST['age']) && !empty($_POST['description'])) {
echo "Please fill in all the fields";
die();}
// Connect
$link = mysql_connect('mysql_host', 'mysql_user', 'mysql_password');
$select = mysql_select_db('mysql_name', $link);
	
if(!is_resource($link)) {
echo "Failed to connect to the server\n";// ... log the error properly
} elseif(!$select) {
echo "Failed to select database";
} else {
// Reverse magic_quotes_gpc/magic_quotes_sybase effects on those vars if ON.
if(get_magic_quotes_gpc()) {
$name = stripslashes($_POST['name']);
$age = stripslashes($_POST['age']);
$description = stripslashes($_POST['description']);
} else {
            $name = $_POST['name'];
	    $age = $_POST['age'];
	    $description = $_POST['description'];
        }

        // Make a safe query
        $query = sprintf("INSERT INTO users (`name`, `age`, `description`) VALUES ('%s', '%s', %s)",
                    mysql_real_escape_string($name, $link),
		    mysql_real_escape_string($age, $link),
		    mysql_real_escape_string($description, $link));

        mysql_query($query, $link)or die(mysql_error());

        if (mysql_affected_rows($link) > 0) {
            echo "Username Added\n";
        }
    }
}
?>
    Code (markup):

     
    DBallerz, Oct 31, 2007 IP
  2. nico_swd

    nico_swd Prominent Member

    Messages:
    4,153
    Likes Received:
    344
    Best Answers:
    18
    Trophy Points:
    375
    #2
    Looks save to me.

    But you could use for example intval() for the age. No need to let the user enter non-numeric data. And you could use preg_replace() or preg_match() to make sure the name only contains characters you want to allow.

    But the way it is right now, there should not be a danger.
     
    nico_swd, Oct 31, 2007 IP
  3. Fash

    Fash Peon

    Messages:
    37
    Likes Received:
    3
    Best Answers:
    0
    Trophy Points:
    0
    #3 
    <form action="<?php $_SERVER[PHP_SELF]; ?>" method="post"><br />
    PHP:
    This line is vulnerable to XSS, but I don't see any SQL injections.
     
    Fash, Oct 31, 2007 IP
  4. nico_swd

    nico_swd Prominent Member

    Messages:
    4,153
    Likes Received:
    344
    Best Answers:
    18
    Trophy Points:
    375
    #4
    How? The user cannot modify the value of the variable. If it was QUERY_STRING, then yes, but not PHP_SELF.

    EDIT:

    Actually, there's no risk at all, because he forgot the echo. :p
     
    nico_swd, Oct 31, 2007 IP
  5. Fash

    Fash Peon

    Messages:
    37
    Likes Received:
    3
    Best Answers:
    0
    Trophy Points:
    0
    #5
    Yes, PHP_SELF is vulnerable..

    http://site.com/index.php/"><script>alert(1)</script> is a perfectly legal URL, and will result in XSS.
     
    Fash, Oct 31, 2007 IP
  6. soulscratch

    soulscratch Well-Known Member

    Messages:
    964
    Likes Received:
    45
    Best Answers:
    0
    Trophy Points:
    155
    #6
    [To Fash]: In that case, should he use htmlspecialchars() around $_SERVER['PHP_SELF'] or just have a value of "" for the action attribute? I'm curious as well.
     
    soulscratch, Oct 31, 2007 IP
  7. Fash

    Fash Peon

    Messages:
    37
    Likes Received:
    3
    Best Answers:
    0
    Trophy Points:
    0
    #7 
    $_SERVER['SCRIPT_NAME']
    PHP:
    :)
     
    Fash, Oct 31, 2007 IP
  8. webrepair

    webrepair Peon

    Messages:
    41
    Likes Received:
    2
    Best Answers:
    0
    Trophy Points:
    0
    #8
    dont trust any variables like that. SANITIZE, SANITIZE, SANITIZE.
     
    webrepair, Nov 1, 2007 IP
  9. mehdiali

    mehdiali Peon

    Messages:
    99
    Likes Received:
    0
    Best Answers:
    0
    Trophy Points:
    0
    #9
    1. because it doesn't have any echo or something like this
    i think it doesn't have any problem.
    2. you can easily use 'htmlentities' , like this :
    /* Filter Input ($name, $comment) */
    $clean_name = htmlentities($_POST['name'], ENT_QUOTES, 'UTF-8');
     
    mehdiali, Nov 2, 2007 IP
  10. exodus

    exodus Well-Known Member

    Messages:
    1,900
    Likes Received:
    35
    Best Answers:
    0
    Trophy Points:
    165
    #10
    exodus, Nov 2, 2007 IP