Jump to content

Archived

This topic is now archived and is closed to further replies.

phil88

Security question, how can this be hacked?

Recommended Posts

I'm learning PHP and am working on the security side of things so I can make my scripts secure, I understand about using various functions like mysql_real_escape_string() etc, but I want to be able to test stuff.


I made a (very) simple login script and was wondering, what could a user of the script do to the form or query from the browser that could;
a) Allow them to login without a password and/or username
b) Allow them to view passwords of users who aren't them

[code=php:0]
<?PHP
if ($_GET['Submit'] == FALSE){
echo "<form method='get' action='login.php'>";
echo "Username: <input type='text' name='username'>";
echo "<br>Password: <input type='password' name='password'>";
echo "<br><input type='submit' value='Submit' name='Submit'>";
echo "</form>";
}else{
mysql_connect("localhost", "user", "pass");
mysql_select_db("usershack");
$username = $_GET['username'];
$password = $_GET['password'];
$query = mysql_query(" SELECT * FROM users WHERE username='$username' AND password='$password' ");
while($row = mysql_fetch_array($query)){
echo $row['username'];
echo "<br>";
echo $row['password'];
}
}
?>[/code]

Like I said, very basic login script, I'm just working from the ground up as far as security goes.

Share this post


Link to post
Share on other sites
an sql injection, you should encrypt your password and addslashes, use md5

Share this post


Link to post
Share on other sites
as r-it said - you're leaving yourself to SQL injections if you do not do any processing to input provided by the user - be it $_GET, $_POST or $_COOKIE (GPC). If you pop 'SQL Injection' into Google, you'll get a better idea of how it works and how to do it yourself, hence learning how to cover your tracks using 'addslashes' and other.
i have a little function i use for everything - $_POST, $_GET, the lot, which to now has worked very well for me:
[code]
<?php
function parse_input($g_p)
{
  $ret = array();
  foreach ($g_p as $key=>$value)
  {
      if (!is_array($value))
      {
        $value = strip_tags($value);
        $ret[$key] = get_magic_quotes_gpc() ? $value : addslashes($value);
      }
      else
      {
        $ret[$key] = parse_input($value);
      }
  }
  return $ret;
}
?>
[/code]
which strips out any html tags and also adds slashes depending on the PHP settings.
basically I just call it like:

[code]
<?php
$post = parse_input($_POST);
$get = parse_input($_GET);
?>
[/code]
and only refer to $_POST and $_GET values via my new $post and $get arrays.

I'm also not keen on having login scripts run on $_GET - basically, your html form presents the user with a password field (ie, starred out instead of showing the characters) but as soon as the form is submitted, the password becomes clear text in the URL. I'd recommend changing the 'action' of the form from 'get' to 'post' and also all the $_GET's to $_POST's.

cheers
Mark

Share this post


Link to post
Share on other sites
Your script is very unsecure, and can lead to sql injection attacks, as r-it mentioned above. To prevent SQL Injection attacks you should consider using mysql_real_escape_string. It would be a good idea to encrypt the users password too, using md5 or sha-1

Also you should use the POST method rather than the GET method, as people will be able to see the username and password in the URL. Whereas POST sends the data in the background.

Share this post


Link to post
Share on other sites
mark is right having them set to get is the stupidest thing you could do.
Always use post on those kinds of things
Also if you just use mysql_real_escape_string();
and whatever else
maybe some extra safety, to make sure things don't go haywire, believe me
strtolower()
will stop all the upercase letters
you can do trim(" variable ")
which will prevent whitespaces
also validate the material
make sure it's not blank, ex cetera
I know most people won't agree, but I feel having  1 login box, not really makes things secure, but it makes it easier to run scripts on login.  I always end up having a lot of scripts, where I have to check for various things for the user and update if necessary, if there is only 1 login area, then you can do that easier, than having it go to a login.php redirrect page every time.

Share this post


Link to post
Share on other sites
Thanks for all the help. I read up on MySQL injections and such and rewrote it, would this be secure?

[code=php:0]<?PHP

if($_POST['submit'] == FALSE){
echo "<form method='post' action='login.php'>
Username: <input type='text' name='username'><br>
Password: <input type='password' name='password'><br>
<input type='submit' value='Submit' name='submit'>
</form>";
}else{
$username = $_POST['username'];
$password = $_POST['password'];
if($username == FALSE){
echo "No username was entered.";
exit;
}elseif($password == FALSE){
echo "No password was entered.";
exit;
}else{
$username = htmlentities(mysql_real_escape_string($username));
$password = sha1($password);
mysql_connect("localhost","phil","apples");
mysql_select_db("usershack");
$query = mysql_query(" SELECT * FROM users
WHERE username='$username'
AND password='$password' ")
or die(mysql_error());
if(mysql_num_rows($query) == 0){
echo "No such user found or login details are incorrect.";
exit;
}else{
echo "Welcome back ".$username."!";
exit;
}
}
}
?>[/code]

Would there be any way to get around the login system or insert something dodgy to retrieve stuff from the database that shouldn't be retrievable? How secure are the $_POST variables? Can they be edited in anyway after the form has been submitted?


Also, once the user is logged in, what would be the most secure way of keeping track of them? Could I just create a session variable that'll be set to true when the user is logged in and false when they're logged out? How easy can $_SESSION variables be manipulated by the user?


Edit: I just read the sticky in this forum about error checking, so I'll sort out the way I've done it so it works better.

Share this post


Link to post
Share on other sites
No matter what you do, or how you do it, if someone wanted to do something bad enough to it, they would do something to it.  That is a fact, where there's a will there's a way, all you can do is work to make it as secure as possible, hope for the best and learn from your mistakes if you ever get hacked.
Even if you tried every possible way to make it  bullet proof, bullets could still get through, nothing you can do, but try your hardest to make it secure, while still having fun.
What you currently have looks pretty secure. but if your password is apples, then someone probably already got it, that's a shitty password, try letter's and numbers, you have to remember if someone get's the apssword they can connect from your database even from another website, unless there was a firewall behind it, and even then they sometimes could.  WHat I suggest, is if you haven't already create a good password.  Something with letters, and numbers, starting with a letter though.

Share this post


Link to post
Share on other sites
Yes much better security wise. Also theres no need to use the htmlenties function with the mysql_real_escape_string.

However the layout of code could be better.

Share this post


Link to post
Share on other sites
[quote]
What you currently have looks pretty secure. but if your password is apples, then someone probably already got it, that's a shitty password, try letter's and numbers, you have to remember if someone get's the apssword they can connect from your database even from another website, unless there was a firewall behind it, and even then they sometimes could.  WHat I suggest, is if you haven't already create a good password.  Something with letters, and numbers, starting with a letter though.[/quote]
Yeah, it's just a password on a test server I have for developing, totally disconnected from the outside world, only I have physical access to it and there's not really much you can do with it even if you do get access to it  ;D

[quote]Also theres no need to use the htmlenties function with the mysql_real_escape_string.[/quote]
Does mysql_real_escape_string escape HTML characters aswell then?

[quote]
However the layout of code could be better.[/quote]
What do you mean?

Share this post


Link to post
Share on other sites
THe only thing mysql_real_escape_string() doesn't do that needs to be done if magic quotes isn't on, is strip slashes for databasing.  and it may even do that.
php.net tells you everything it does on them.

Share this post


Link to post
Share on other sites
Would using the function on php.net be a better choice than using mysql_real_escape_string?
This function;
[code=php:0]function quote_smart($value)
{
  // Stripslashes
  if (get_magic_quotes_gpc()) {
      $value = stripslashes($value);
  }
  // Quote if not a number or a numeric string
  if (!is_numeric($value)) {
      $value = "'" . mysql_real_escape_string($value) . "'";
  }
  return $value;
}[/code]

Share this post


Link to post
Share on other sites
To stop html you should use strip_tags which will remove any html in a string. Yes that'll be better just incase a setting called magic quotes is enabled.

Share this post


Link to post
Share on other sites
[quote author=wildteen88 link=topic=102152.msg405348#msg405348 date=1154115056]
To stop html you should use strip_tags which will remove any html in a string.
[/quote]
The idea wasn't so much to remove HTML, just to stop it being treated as HTML. I mean, some people would like their usernames to be <Username> for example, but if that was parsed as HTML, it would be invisible as it's between < and >.

Share this post


Link to post
Share on other sites
mysql_real_escape_string includes the htmlentities function so there will be no need to use it again.

Share this post


Link to post
Share on other sites
i use this. This may not be the most secure but it suits my purposes

[code=php:0]
<?php
include ('includes/db.php');
if (isset($_POST['submit'])) {
    array_pop($_POST);
    if ( get_magic_quotes_gpc() ) {
        $_POST= array_map('stripslashes', $_POST);
    }
    if ((!$username) || (!$password)) {
        echo"You did not enter the following required information";
        if (!$username) {
            echo "User name is a required field";
        }
        if (!$password) {
        echo "Password is a required field";
        }
        exit;
    }  
    $username= mysql_real_escape_string(trim($_POST['username']));
    $password= mysql_real_escape_string(trim($_POST['password']));
    $mdpwd= md5($password);

    $sql= sprintf("SELECT COUNT(*) AS login_match FROM `users` WHERE `username` = '%s' AND `password`= '%s'", $username, $mdpwd);
    $res= mysql_query($sql) or die(mysql_error());
    $login_match= mysql_result($res, 0, 'login_match');

    if ( $login_match == 1 ) {
         //loged in
    } else {
    // not logged in
}
}
//put your log in form here
?>[/code]

maybe this will give you some ideas.  For some reason the sql gets all messed up when pasting it here but this should give you a rough idea.

Share this post


Link to post
Share on other sites
What code are you using on your other php pages to ensure that users cannot bypass your login screen?

If you aren't already I would recommend using sessions. In your login code, add something like this:

[code=php:0]
if(mysql_num_rows($query) == 0){
echo "No such user found or login details are incorrect.";
exit;
}else{
                                      session_start();
                                      $_SESSION['username'] = $username;

echo "Welcome back ".$username."!";
exit;
}
[/code]

and on each page that you want to prevent unauthenticated users from accessing, add this around your page code

[code=php:0]
<?php

// initialize the session
session_start();

// Check if the user session was created
if(isset($_SESSION['username'])){

// YOUR CODE HERE
// YOUR CODE HERE
// YOUR CODE HERE

}
else{
// direct the user to the login page
Header("Location: /login.php");
}

?>
[/code]


Share this post


Link to post
Share on other sites
this is a much better/ faster way of processing a login than [code=php:0]mysql_num_rows[/code]

[code=php:0]
$sql= sprintf("SELECT COUNT(*) AS login_match FROM `users` WHERE `username` = '%s' AND `password`= '%s'", $username, $mdpwd);
$res= mysql_query($sql) or die(mysql_error());
$login_match= mysql_result($res, 0, 'login_match');

if ( $login_match == 1 ) {
    session_start();
    $_SESSION['username'] = $username;
    echo "Welcome $username you are now loged in";
} else {
   echo "Your username/password do no match";
   // not logged in
}[/code]

And as far as the sessions at the top of each page I would do this.

[code=php:0]
session_start();
if (!$_SESSION['username']) {
  echo "You must log in to view this page";
  include('yourform.php");
  exit;
}[/code]

Share this post


Link to post
Share on other sites
it's good practice to do section even above the db calls.
the connection and everything
it should be above everything else
as far as
sprintf("
I disagree with that usage, but it's partially personal preference.

Share this post


Link to post
Share on other sites
tomfmason:
I don't understand this line of your code;
$login_match= mysql_result($res, 0, 'login_match');

How can there be a row 0, and what should 'login_match' be if there isn't a field in the database called that?

HeyRay2:

Yes, I was going to use Sessions, but I have a question, how easily can they be modified by the user? I mean, would I need to run some sort of validation on them or could I just do a simple if(isset($_SESSION['logged_in']) to check if the user is logged in? (Assuming I set $_SESSION['logged_in'] when they log in)

Share this post


Link to post
Share on other sites
[quote author=phil88 link=topic=102152.msg405480#msg405480 date=1154177295]
Yes, I was going to use Sessions, but I have a question, how easily can they be modified by the user? I mean, would I need to run some sort of validation on them or could I just do a simple if(isset($_SESSION['logged_in']) to check if the user is logged in? (Assuming I set $_SESSION['logged_in'] when they log in)
[/quote]

I am also interested in this piece of information as well. Apart from "other people on a shared server could see your sessions" type issues, is there any other security risk? No session id's are passed in my urls ever... I just use a $_SESSION['level'] (holds their access leve) to reveal certain parts of a site (ie special downloads, special news, admin functions etc).

Is that secure or can a user some way manipulate it. As usual, session_start() on each page, and then a check to see if they have the required level to see it. No cookies set afaik, just session variables. When browser closed, they are "logged out". Also, I didn't see a way to set a time limit on sessions like this - is there one? Or is it a php.ini alteration only? As far as I know to date it's secure, but with this topic tackling sessions and their security just now, I thought it would be a good idea to ask about it as well :)

Share this post


Link to post
Share on other sites
[quote]Is that secure or can a user some way manipulate it. As usual, session_start() on each page, and then a check to see if they have the required level to see it. No cookies set afaik, just session variables. When browser closed, they are "logged out". Also, I didn't see a way to set a time limit on sessions like this - is there one? Or is it a php.ini alteration only? As far as I know to date it's secure, but with this topic tackling sessions and their security just now, I thought it would be a good idea to ask about it as well Smiley[/quote]
Well I guess one way to get around the time issue would be to set a cookie when the session is set, to expire after a certain amount of time. Then on each page you could call a function or something that'd check for the cookie, if the cookie is expired/isn't there, then destroy the session.

Share this post


Link to post
Share on other sites
[quote author=phil88 link=topic=102152.msg405480#msg405480 date=1154177295]
tomfmason:
I don't understand this line of your code;
$login_match= mysql_result($res, 0, 'login_match');

How can there be a row 0, and what should 'login_match' be if there isn't a field in the database called that?

HeyRay2:

Yes, I was going to use Sessions, but I have a question, how easily can they be modified by the user? I mean, would I need to run some sort of validation on them or could I just do a simple if(isset($_SESSION['logged_in']) to check if the user is logged in? (Assuming I set $_SESSION['logged_in'] when they log in)
[/quote]

You do not need a field in your database called login_match. Just try the code and you will see that it works and processes the login faster then most other login scripts. See my post on the pervious page for the entire script

BusinessMan,
                why do you disagree with the usage of [code=php:0]sprintf()[/code] in the case.



By the way Business man is right about the sessions I would start the session at the very top of the script.

Share this post


Link to post
Share on other sites
[quote]
You do not need a field in your database called login_match. Just try the code and you will see that it works and processes the login faster then most other login scripts. See my post on the pervious page for the entire script[/quote]
I'm sure it does work. I'm just curious as to know how it works and how it would be better than the standard mysql_num_rows way, is that function more resource intensive or something?

Share this post


Link to post
Share on other sites
ok well this is faster manly because of the way that I choose to search the database

[code=php:0]$sql= sprintf("SELECT COUNT(*) AS login_match FROM `users` WHERE `username` = '%s' AND `password`= '%s'", $username, $mdpwd);[/code]

SELECT COUNT is a faster way of searching a database when counting how many matches you have. If you try to use [code=php:0]mysql_num_rows[/code] with SELECT COUNT then you will always return a value of 1 or a sucessful login. That is the reason I that I use mysql_result

Now I will explain [code=php:0]mysql_result[/code]

well here is what the manual says about it

string mysql_result ( resource result, int row [, mixed field] )
result
The result resource that is being evaluated. This result comes from a call to mysql_query().

row
The row number from the result that's being retrieved. Row numbers start at 0.

field
The name or offset of the field being retrieved.

It can be the field's offset, the field's name, or the field's table dot field name (tablename.fieldname). If the column name has been aliased ('select foo as bar from...'), use the alias instead of the column name. If undefined, the first field is retrieved

Good luck,
Tom

Share this post


Link to post
Share on other sites
So with mysql_result, it'll return 1 if it finds a matching record, but if it doesn't find a matching record, it'll be false?

Share this post


Link to post
Share on other sites

×

Important Information

We have placed cookies on your device to help make this website better. You can adjust your cookie settings, otherwise we'll assume you're okay to continue.