Take the 2-minute tour ×
Information Security Stack Exchange is a question and answer site for Information security professionals. It's 100% free, no registration required.

I want to write a login form, and I got one example from the web. I want to know, if there is any SQL injection for this code? If there is, what could the exploit's web form entry look like?

This is my form:

<form method="post" action="">
    <dt class="title"><label for="username">name:</label></dt>
    <dt><input type="text" name="username" id="username"  size="50"></dt>
    <dt class="title"><label for="password">pass:</label></dt>
    <dt><input type="password" name="password" id="password" size="50"></dt>
    <input type="hidden" name="post" value="1" />
    <input type="submit" name="submit" value="submit" class="button">
</form>

and this is my check:

if (($post[username]) AND ($post[password])) 
    {
        $query = 'SELECT * FROM `config` WHERE `config_admin_username`="'.$post[username].'" AND `config_admin_password`=MD5("'.$post[password].'") LIMIT 1';
        $sql_user_check = $db->fetch($query);
        if (!$sql_user_check) { 
            $error .= "Wrong Entry.<br />";
        } else {
            $_SESSION[admin] = 1;
            header("Location:index.php");
            exit;
        }
    }

I'm using phpmyadmin 3.2.0.1 on wamp 2.0i

share|improve this question
1  
If you are new to PHP, I would not recommend writing your own logon form. I would try to plug in some framework or at least find an example where the author stresses security in the title. You should of course now how to look for faults, but there are many complexities and bad things that can happen even beyond SQL injection from a logon form. –  Eric G Apr 22 '13 at 3:09

3 Answers 3

up vote 18 down vote accepted

You've come to the right place. Welcome to IT security!

is there any sql injection for this code?

Yes

if there is what is that Entry?

username: [any username from your website]" /*
password: sux0r")*/ OR ("1"="1

It will run this query:

SELECT * FROM `config` 
WHERE `config_admin_username`="[any username from your website]" /* 
      AND `config_admin_password`=MD5("sux0r")*/ OR ("1"="1") LIMIT 1'

If we remove the commented-out parts that SQL engine won't parse, this results in:

SELECT * FROM `config` 
WHERE `config_admin_username`="[any username from your website]" OR ("1"="1") LIMIT 1'

SQL query will be parsed up till the commented out part (I used the /* and */ start-comment/end-comment pair in my example and assuming you're using MySQL, but that can be different depending on RDBMS used), effectively disabling your password check altogether. Even if in-SQL commenting is not supported, or disabled, the options to exploit your login are really limitless. I won't even go into your choice of MD5 as a password hashing algorithm, as you have a lot of other things to consider first. It clearly isn't the recommended hash algorithm for password hashing though.

You've done good to have come to this website and that you're asking yourself questions regarding your code safety. Congratulations, you're already a better PHP programmer than most that never got so far are! Now, it's up to you what you'll learn from already existing questions and answers here. Some tags to consider are , , , ,...

share|improve this answer
    
tnq for reply , but stil script give me Wrong Entery. –  sorroshsaket Apr 21 '13 at 23:12
1  
@sorroshsaket - OK I've checked the code that you linked to, but I can't open the example page (doesn't matter really). I would avoid these SQL injection examples on a live website anyway, unless you own it. The code is not SQL injection safe. For obvious reasons, I will reserve my further comments on where exactly it has weak points, but it should be clear to anyone that calls himself a programmer these days, if they bother to check the source code. The only attempt at input parameter sanitization is declared in cleaner function in include\funks.php. It is easily exploitable! –  TildalWave Apr 22 '13 at 11:03
1  
@sorroshsaket - using scripts developed by others is fraught with danger if you don't understand them. Linking them here is also not ideal, and under some regulations could be construed as aiding an attack (see the EFF's comments on the latest proposed US regs) so I have deleted those comments. –  Rory Alsop Apr 22 '13 at 12:11
1  
@TildalWave - Congratulations, you gained the endless patience award! –  martinstoeckli Apr 26 '13 at 7:58
1  
@trusktr - That is correct. I also suggest you read up on other questions and their answers tagged sql-injection. If however you have an additional question that was not previously covered, please post it as a new question. Thanks! –  TildalWave Sep 24 '13 at 22:41

Yes it is. You are not validating anything. You have to use

  • mysql_real_escape_string to prevent SQL injection
  • htmlentities/htmlspecialchars to prevent XSS attacks

Take a look at this tutorial as it helps on learning to prevent SQL injection.

share|improve this answer
    
Would the down voter be prepared to explain why? Is the linked tutorial not good (or was the down vote for grammar??!) ? I am starting for a position of knowing little about the subject, so can't judge the quality of the information. Thanks. –  S List Apr 26 '13 at 9:30
    
Wouldn't know about the reasons for the down-vote, but it's a general consensus (and the practice has proven it) that using mysql_real_escape_string alone isn't sufficient. See @David's answer and also links in my comment below it. For more on the subject of sanitizing input parameters, and mitigating all kinds of other possible threats, OWASP should always be high on the list too. The blog you attached the link to isn't sufficiently covering this topic, tho. There are no magic bullets in IT Security. :) –  TildalWave Apr 26 '13 at 14:06

TildalWave has a great answer, but there is a great mediation to SQL injection that was missing from the answer. Prepared Statements. As Tildal said the entire statement with user input is parsed with no parsable distinction between variables and constant parts of the statement. The solution is sending the constant part of the statement with variable placeholders then the variables. PDO is what I use for my SQL interaction. Here is the function I use...

function _SQL_QUERY($_base_request, $_request_arguments)
{
                $_DB_USERNAME = "bob"; $_DB_PASSWORD="secret"; $_DB_LOCATION="localhost"; $_DB_NAME="MYWICKEDDATABASE"; $_DB_SERVER_TYPE="mysql";
                if(!isset($_DATABASE_CONNECTION))
                {
                        $_DATABASE_CONNECTION = new PDO("$_DB_SERVER_TYPE:dbname=$_DB_NAME;host=$_DB_LOCATION;charset=utf8", $_DB_USERNAME, $_DB_PASSWORD);
                        $_DATABASE_CONNECTION->setAttribute(PDO::ATTR_EMULATE_PREPARES, false);
                }
                $__STMT = $_DATABASE_CONNECTION->prepare($_base_request);
                if($__STMT->execute($_request_arguments))
                {
                        return $__STMT;
                }
                else
                {
/*
        //Helpful ONLY when debugging!!!
                        echo 'ERROR</ br>';
                        echo $_base_request;
                        echo '</ br>';
                        print_r($__STMT->errorInfo());
                        print_r($_request_arguments);
                        die('</ br>');
*/
                        return false;
                }
}

to use it you would just need to run this

_SQL_QUERY("Select * FROM names WHERE name=:var1 AND age=:var2", array('var1'=>'jim','var2'=>50));

There are plenty of improvements to be made (mainly functionaly) to this but it is a vast improvement to the traditional SQL functions. BTW its important to know that if a row has a failed attempt at an injection in it's data, a backup and restore may actually make it successful depending on the method used. Kudos for doing a security audit of your code, if only more people learned programming with that motivation towards security.

share|improve this answer
    
this is very nice & i read about it . I like it , but there are many way to stop injection like set input on this function: mysql_real_escape_string($post[username]) but i want to show this team there is a big hole on their login form but when i inject the query " will change to &qout; and i have no idea for this –  sorroshsaket Apr 22 '13 at 7:49
    
David thanks for your answer! @sorroshsaket - mysql_real_escape_string alone isn't safe enough to prevent SQL injections, and prepared statements are one suggested way to prevent them, on top of string escaping. Both of it together are considered input parameter sanitization, where string escaping can be considered normalization only and not enough. See this answer, also these two: 1, 2. –  TildalWave Apr 22 '13 at 10:56

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.