Page 1 of 2

Is this Secure?

Posted: Mon Nov 07, 2005 1:21 pm
by Dark[NSF]
I'm aware that absolutely nothing is encrypted, but besides that, is this a security flaw?


// My included security check function.

Code: Select all

// Return true if logged in.
function bverify_login($bIsAdmin = 0)
{
  global $logins_table;

  if ( (isset($_COOKIE["tj_email"])) && (isset($_COOKIE["tj_password"])) )
  {
    $input_email = $_COOKIE["tj_email"];
    $input_password = $_COOKIE["tj_password"];
  }
  else if ( (isset($_POST["login"])) && (isset($_POST["password"])) )
  {
    $input_email = $_POST["login"];
    $input_password = $_POST["password"];
  }
  else
    return false;

  $result = mysql_query(" SELECT * FROM $logins_table WHERE email = '$input_email' ");
  $found = mysql_fetch_row($result);

  if ( $found[12] != '') // is activated?
    return false;

  if ( $found[2] == $input_password )
  {
    if ( $found[10] >= $bIsAdmin )
      return true;

    return false;
  }

  return false;
}

// This is before the header include in every page that needs protection, this particular example checks for admin rights, checking for user rights is 0.

Code: Select all

if ( !bverify_login(1) )
    header("Location: index.php?message=no_permissions");

Posted: Mon Nov 07, 2005 2:24 pm
by Ambush Commander
It's not secure. It stores the password in a cookie as cleartext. There are other issues, but we'll talk about this one first.

Posted: Mon Nov 07, 2005 2:44 pm
by Weirdan
Ambush Commander wrote:It's not secure. It stores the password in a cookie as cleartext. There are other issues, but we'll talk about this one first.
Personally, I would rate possible sql injection as much more severe flaw comparing to plain-text password in cookies. :twisted:

Posted: Mon Nov 07, 2005 2:47 pm
by Ambush Commander
Ah, you're right. Missed that. Hmph... that's why you should always use bind sql. Clears a bunch of messy problems that occur when you forget to escape output.

Posted: Mon Nov 07, 2005 3:20 pm
by Dark[NSF]
Ambush Commander wrote:It's not secure. It stores the password in a cookie as cleartext. There are other issues, but we'll talk about this one first.
Yes, are you refering to encryption? I stated in my post that I'm already aware of this. What are some of the other issues?

Posted: Mon Nov 07, 2005 3:24 pm
by Ambush Commander
As stated above, SQL injection. input_email isn't escaped, so an attacker can execute an arbitrary SQL statement.

Posted: Mon Nov 07, 2005 3:29 pm
by Dark[NSF]
Ambush Commander wrote:As stated above, SQL injection. input_email isn't escaped, so an attacker can execute an arbitrary SQL statement.
how would i go about doing this? is it a php function?

Posted: Mon Nov 07, 2005 3:38 pm
by Dark[NSF]
i good place to start is probably Strip_Tags(), I'm not sure how I can filter SQL statements however.

Posted: Mon Nov 07, 2005 3:41 pm
by Ambush Commander
strip_tags doesn't have anything to do with it.

Try... mysql_real_escape_string()

Posted: Mon Nov 07, 2005 3:44 pm
by Dark[NSF]
Ambush Commander wrote:strip_tags doesn't have anything to do with it.

Try... mysql_real_escape_string()

for SQL it doesn't, but it will prevent some javascript on random html tags, or some XSS.



ahh, this will help alot. Is there any other serious and obvious issues i should be worried about?

Posted: Mon Nov 07, 2005 3:48 pm
by Ambush Commander
Well, it looks like you've got a fairly large application, so consider using a Database wrapper. It will help prevent future SQL injection attacks.

Once again, don't put passwords as cleartext in cookies. Instead, use remember me tokens (no encryption required).

Posted: Mon Nov 07, 2005 3:49 pm
by foobar
Dark[NSF] wrote:
Ambush Commander wrote:strip_tags doesn't have anything to do with it.

Try... mysql_real_escape_string()

for SQL it doesn't, but it will prevent some javascript on random html tags, or some XSS.
Which has nothing to do with your question. Ambush commander already gave you the answer.

Posted: Mon Nov 07, 2005 3:54 pm
by Dark[NSF]
foobar wrote:
Dark[NSF] wrote:
Ambush Commander wrote:strip_tags doesn't have anything to do with it.

Try... mysql_real_escape_string()

for SQL it doesn't, but it will prevent some javascript on random html tags, or some XSS.
Which has nothing to do with your question. Ambush commander already gave you the answer.

I was refering to security as a whole, strip_tags() is probably a good place to start. i'm not saying he's wrong.

Posted: Mon Nov 07, 2005 4:08 pm
by Ambush Commander
A login system also needs to manage sessions. Can you post that code for a security audit too?

Posted: Mon Nov 07, 2005 4:13 pm
by Dark[NSF]
Ambush Commander wrote:A login system also needs to manage sessions. Can you post that code for a security audit too?
I was wondering if sessions were necessary, my current system doesn't use sessions. only cookies. i suppose that is a bad thing.



good thing this forum exists.