To register for an Internet.com membership to receive newsletters and white papers, use the Register button ABOVE.
To participate in the message forums BELOW, click here
Code CritiqueHaving someone critique your code is always a great way to hone the skills. Stop in and post your code to see what your peers may have done differently.
Hi Folks.
This is a log in page that I coded today. The concept behind the database is:
1. One database contains user names, passwords and a surveyID
2. The other database contains the URL to the survey.
Which the reason for the two queries.
I believe this works as it stands, but would like some opinions and ideas.
The only part I haven't completed would be an error message of sorts, if the username and pw is wrong.
For that I was thinking of using another switch on $result from the first query.
Thanks for any suggestions you may have.
PHP Code:
<?php
$log_in = $_POST['Submit'];
$FormUserName = $_POST['FormUserName'];
$FormUserPass = $_POST['FormUserPass'];
//--Clean Data
$FormUserName = strip_tags($FormUserName);
$FormUserPass = strip_tags($FormUserPass);
$LFormUserName = strlen($FormUserName);
$LormUserPass = strlen($FormUserPass);
$authorized = '0';
//-Check for existence of input from user
if($log_in)
{
switch($log_in)
{
case !$FormUserName || !$FormUserPass:
$error = "Please fill out both fields";
break;
//--Leave commented out unless there is a length restriction on user input --//
//case $LuserName < 5:
// $error = "Your username is too short";
//break;
//case $LuserPass < 5:
// $error = "Your password is too short";
//break;
case $FormUserName && $FormUserPass:
require_once("dbincludes/db_connect.inc.php");
mysql_connect($db_server, $db_user, $db_password) or die("Cannot connect");
mysql_select_db($db_database) or die("Could not choose database");
$query = mysql_query("select * from $db_user_table where userName = '$FormUserName' and userPass = '$FormUserPass' ");
$result = mysql_num_rows($query);
while($userInfo=mysql_fetch_array($query))
{
$DBUserName = $userInfo['userName'];
$DBUserPass = $userInfo['userPass'];
$surveyID = $userInfo['surveyID'];
}
Location: Rapid Offensive Unit "Foreign Object Damage"
Posts: 19,128
But what if (I claim) my username and password are ' OR ''='?
__________________
On two occasions I have been asked [by Members of Parliament], "Pray, Mr. Babbage, if you put into the machine wrong figures, will the right answers come out?" I am not able rightly to apprehend the kind of confusion of ideas that could provoke such a question.
//--Clean Data
//--Strip HTML tags and strip white space from beg. and end.
$FormUserName = strip_tags($FormUserName);
$FormUserPass = strip_tags($FormUserPass);
$FormUserName = trim($FormUserName);
$FormUserName = trim($FormUserPass);
$FormUserName = ltrim($FormUserName);
$FormUserName = ltrim($FormUserPass);
While researching, I noticed that idea 1 would not be practical.
Thanks for not giving a direct answer, I enjoy trying to figure it out.
And thanks for the direction.
Does this code seem more appropriate?
Would running the ereg() function be suitable for spitting out an error if HTML tags are used as a username or pw?
I know it wouldn't strip them out, but I don't think it would be good to accept anything if it had an HTML tag.
~Az
Originally posted by Azazeal After even more research, my ereg() is incorrect.
I need to search the string for illegal characters and then spit an error out.
Is that correct?
searching for NOT legal chars works too
__________________
-Karl Problem solved? Mark your thread resolved!
See how many times my link has been used to mark a thread resolved here
(fixed to work with updated VBulletin... this works again!)
Belongs in the ranting thread...good newbs...cool code there AZ!
__________________
"A proof is a proof. What kind of a proof? It's a proof. A proof is a proof. And when you have a good proof, it's because it's proven." -- Jean Chrétien
Location: Rapid Offensive Unit "Foreign Object Damage"
Posts: 19,128
Huzzah! We need to recognise the good ones out there more often!
Re: the code. It only works for things like passwords, but it's pretty good to md5() whatever they enter before storing it in the database. Buncha reasons:
(Assuming the conventional representation of an MD5 hash) the field fixed at char(32);
people can enter pretty much whatever ****e they like into the password field and the resulting hash will be safe to go into the query
Someone gets ahold of your user database they still have work ahead of them to get the users' passwords
Obviously it's only good for passwords and the like where you don't ever need to see the things yourself.
__________________
On two occasions I have been asked [by Members of Parliament], "Pray, Mr. Babbage, if you put into the machine wrong figures, will the right answers come out?" I am not able rightly to apprehend the kind of confusion of ideas that could provoke such a question.
That was from memory, so it may be off a little.
I tried using it in the switch($log_in), but it gave some unexpected results.
PHP Code:
switch($log_in)
{
case !$FormUserName || !$FormUserPass:
$error = "Please fill out both fields";
break;
case ereg('[!@#$%^&*()_+-=[]\{}|;':",./<>?]', $FormUserName):
$error = "Please enter your user name again."
}
I believe is close to what I tried. I have the code at work.
Regarding the md5(), I plan to use that, but I am just taking it step-by-step. I figure I would get the standard validation working, then move to the query again.
The real fun will be coding the admin page, where the admin can add, delete or modify users and survey URL's.
Location: Rapid Offensive Unit "Foreign Object Damage"
Posts: 19,128
It may be better to rewrite the validity check to make sure that users only use characters they are allowed to use, and let them use only safe characters; phrase the validation task as "Have you followed these rules? NO? Then go and try again." and phrase the rules in a positive fashion: "Passwords are to be at least five characters and may use upper- or lowercase letters (a-z), digits (0-9), spaces, or underscores (_)". The test for that would be
PHP Code:
if(preg_match("/[^a-z0-9_ ]/i",$userName))
{ //Invalid characters found
}
or
PHP Code:
if(eregi("[^a-z0-9_ ]",$userName))
{ //Invalid characters found
}
Limit your exposure to what you know is safe, and don't allow anything through that doesn't pass muster; instead of exposing yourself to everything and then trying to second-guess every possible exploit.
It's a general security principle, and one that Microsoft would be well-advised to take on board....(I mean, who actually uses that Indexing Services thingy?)
__________________
On two occasions I have been asked [by Members of Parliament], "Pray, Mr. Babbage, if you put into the machine wrong figures, will the right answers come out?" I am not able rightly to apprehend the kind of confusion of ideas that could provoke such a question.
I believe I tried the above, which I assume is the same as the eregi() that you posted.
However, when I tried a username like ' !az ', it was allowed through, since the string contained 'az' in it.
It probably was the coding, so I will give it a go tomorrow.
Thanks for the suggestion.
I believe I tried the above, which I assume is the same as the eregi() that you posted.
However, when I tried a username like ' !az ', it was allowed through, since the string contained 'az' in it.
It probably was the coding, so I will give it a go tomorrow.
Thanks for the suggestion.
~Az
notice the placement of your []'s as opposed to Weedspackets []'s
in regexp, [text here] designates a list of characters... to cocmbine in 1 type of check.. ie [^A-Z0-9] means a range of text thats NOT A-Z (all uppers) or 0-9 (numerics) where as....
^[A-Z0-9] means any line STARTING with A-Z (all uppers) or 0-9 (numerics)
2 totally different contexts for that flag in regexps
(Weed, correct me if I'm wrong... but I feel fairly confident in my understanding of that atleast )
__________________
-Karl Problem solved? Mark your thread resolved!
See how many times my link has been used to mark a thread resolved here
(fixed to work with updated VBulletin... this works again!)
$LFormUserName = strlen($FormUserName);
$LFormUserPass = strlen($FormUserPass);
$UserNameLength = '5';
$UserPassLength = '5';
$authorized = '0';
//***************************NOTHING BELOW THIS LINE SHOULD BE CHANGED********
//-Check for existence of input from user
if($log_in)
{
if(ereg('[^a-z0-9_ ]', $FormUserName))
{
$error = "You have used an invalid character in your user name.<br> Valid characters consist of upper or lower case letters (a-z), numbers (0-9), spaces or underscores (_).";
}
//--Validate Data
switch($log_in)
{
case $FormUserName && !$FormUserPass:
$error = "Please enter your password";
break;
case !$FormUserName && $FormUserPass:
$error = "Please enter your username.<br>Your password has been cleared for security reasons.";
break;
case !$FormUserName || !$FormUserPass:
$error = "Please fill out both fields";
break;
//--Leave commented out unless there is a length restriction on user input --//
//case $LFormUserName < $UserNameLength:
// $error = "Your username is too short";
//break;
case $LFormUserPass < $UserPassLength:
$error = "Your password is too short.<br>The required length is five characters.";
break;
case $FormUserName && $FormUserPass:
require_once("dbincludes/db_connect.inc.php");
mysql_connect($db_server, $db_user, $db_password) or die("Cannot connect");
mysql_select_db($db_database) or die("Could not choose database");
$query = mysql_query("SELECT * FROM $db_user_table
WHERE userName = '$FormUserName'
AND userPass = '$FormUserPass' ");
$result = mysql_num_rows($query);
while($userInfo=mysql_fetch_array($query))
{
$DBUserName = $userInfo['userName'];
$DBUserPass = $userInfo['userPass'];
$surveyID = $userInfo['surveyID'];
}
$authorized='1';
}
}
if($authorized=='1')
{
$query = mysql_query("SELECT * from $db_survey_table
WHERE surveyID = '$surveyID' ");
while($d=mysql_fetch_array($query))
{
$url = $d['URL'];
header("Location: $url");
}
}
Here is the latest code.
Thanks for the help, it works a charm.
Could you still review what I have here, as any other suggestions would be useful?
Linear Mode
