PDA

View Full Version : [php] Password



NeoDreamer
January 30th, 2007, 02:10 PM
Is this password form secure? If not, what do I need to check for?


<?php
function secCode()
{
include_once("constants.php");

// obtain the password the user has entered
$password = $_POST['password'];

// warn for incorrect password
if(($password != "321498762") && (strlen($password) > 0))
$wrongPW = "<br /><font color=\"$WARN\">Wrong password.</font>";

// ask for password
if($password != "321498762")
{
echo("
<form name=\"form\" method=\"post\" action=\"index.php?sec=admin\">
<p>
Password:
<input name=\"password\" value=\"$password\" type=\"password\" size=\"15\" maxlength=\"15\" />
<input type=\"submit\" name=\"Submit\" value=\"Submit\" />
</p>
$wrongPW
</form>
");
}

// correct password has been entered
else
{
echo(success);
}
}
?>

bwh2
January 30th, 2007, 02:26 PM
man, that's ugly. you should really try using concatenation with single quotes instead of double quotes. it prevents you from needing all those escape characters and is faster as well.

anyway, why are you storing your password in the file? you should store it externally in a non-public location (preferably a mysql db). also, you shouldn't store the password itself but the md5 hash value of the password. then compare md5($_POST['password']) to the md5 of your password.

also, you should be checking for positive matches to verify authentication. not the other way around.

NeoDreamer
January 30th, 2007, 02:32 PM
By single quotes, do you mean this?


echo("<table width='5' height='6'></table>");


I don't understand your "other way around" logic. Isn't !TRUE == FALSE and !FALSE == TRUE. So what's the dealio, yo?

bwh2
January 30th, 2007, 03:10 PM
by single quotes, i mean this:


// fast clean
echo 'here is my image: <img src="'.$imgURL.'" />';
echo 'more text';

// slow and ugly
echo "here is my image: <img src=\"$imgURL\" />";
i actually wrote about it in this blog post (http://brianhaveri.com/?id=34).

anyway, i think the way you have the logic coded just makes it confusing. i think it's better to say, "does this password match? if no, then login failed." rather than, "does this password not match? if no, then login successful."

kenisfam
January 30th, 2007, 03:19 PM
Meh ,
i think its crap.

first of all functions are to put something in it, and also to not return output in the function
rather return a var or array or a boolean

so better would be:



function LoginForm()
{
$form = '';
$form = '<form ...etc';

return $form;
}

function LoginCheck($_PostArray)
{
return ($_PostArray['value'] == "password") ? true : false;

/*
// or more adv
$errors = array();
if ($_PostArray['value'] != "password")
{
$errors[]="password dont match";
}
// add more

if (isset($errors) && count($errors) > 0 )
{
return $errors;
}else{
return true;
}
*/
}



if ($_SERVER['REQUEST_METHOD'] == 'POST')
{
if (LoginCheck($_POST) === TRUE)
{
// valid
}else{
// not valid
}
}else{
// show form
echo LoginForm();
}

bwh2
January 30th, 2007, 03:29 PM
yeah, ideally you would be doing something like ^ (or just use PEAR::Auth which basically does that)

NeoDreamer
January 30th, 2007, 04:19 PM
So is there any way for them to obtain the password if I keep it inside the PHP file?

bwh2
January 30th, 2007, 04:25 PM
sure. for instance if you have an upload form somewhere else and don't protect against .php file uploads. so someone uploads a php file (which has code to display the PHP code from your password page), goes to that address, and like magic there's your code in full view with password and all.