Germaris Posted December 13, 2012 Share Posted December 13, 2012 (edited) Hi there! I wish to output an array. In my PHP Script, I wrote this: function synchrouser($user,$pass) { $user = trim($user); $pass = md5(trim($pass)); $table = '_a_users'; $query ="SELECT * FROM $table WHERE userName='$user' AND userPassword='$pass'"; $myresult =mysql_query($query); $mydata = ""; while ($row =mysql_fetch_array($myresult)) { $lastName = $row["lastName"]; $firstName = $row["firstName"]; $school = $row["school"]; $promoYear = $row["promo"]; $exitYear = $row["promoOut"]; $promoNumber = $row["promoNum"]; $email = $row["email"]; //insertion of a "|" delimiter $mydata .= "|".$lastName."|".$firstName."|".$school."|".$promoYear."|".$exitYear."|".$promoNumber."|".$email; print "&mydata=".$mydata; } } What's wrong in this coding? Many thanks in advance for your help! Edited December 13, 2012 by Germaris Quote Link to comment Share on other sites More sharing options...
Christian F. Posted December 13, 2012 Share Posted December 13, 2012 Besides the fact that the passwords are insufficiently secured, and modified to boot, I can't tell you what's wrong with it. Primarily because I don't know what you're expecting, nor what is actually happening. If you get any error messages, please post them here as well. (Just remember to remove any usernames from the paths, if present.) $pass = md5(trim($pass)); This is the "bad code" I was talking about. You should be using a user-specific salt, a better hashing algorithm (which hasn't been broken since 2006), and preferably using a multi-pass hashing function. I strongly recommend that you read this article about secure login systems, which will explain everything you need to know about all of this. Quote Link to comment Share on other sites More sharing options...
Germaris Posted December 13, 2012 Author Share Posted December 13, 2012 Thank you very much, Christian, for replying. Besides all security concerns which (as important as they are for sure), I thought I was clear enough with the simple title I've chosen... My problem is : "From this sql query, how can I output an array containing all the results of the query in the order I enumerated them and separated with the specified delimiter?" Quote Link to comment Share on other sites More sharing options...
Christian F. Posted December 13, 2012 Share Posted December 13, 2012 According to the code I see, you are. Now, since you obviously are having some issues with this, we need to know exactly what those issues are. We know what you want, but we have no idea on what the problem is. Not unless you tell us. PS: I recommend that you read this article. Quote Link to comment Share on other sites More sharing options...
mrMarcus Posted December 13, 2012 Share Posted December 13, 2012 (edited) $pass = md5(trim($pass)); You should never mess with a user's password. A space is perfectly valid within a password. However, what OP is doing is removing any lead and trail spaces from the user's password. Basically, you're tampering with the user's password which will cause them to not be able to login next time around. echo md5(' '); // 7215ee9c7d9dc229d2921a40e899ec5f Always give warning to your user's, during registration, as to what you allow/require them to enter when creating their passwords. But don't be stingy and not allow them to use special characters or anything of the like. So, remove the trim() and hash the password straight up: $pass = md5($pass); *That's all I have until you explain exactly what it is you need help with, as Christian F. pointed out. Edited December 13, 2012 by mrMarcus Quote Link to comment Share on other sites More sharing options...
Germaris Posted December 13, 2012 Author Share Posted December 13, 2012 Thanks for your interest in my concerns. I simply have no result at all when using this function inside a very larger script grouping all the functions I use to make a Flash SWF to communicate with a MySQL Database. I made a simple script for testing purpose using the same function I posted here and... it works! You can have a look at the result at: http://www.notre-annuaire.com/strict/try_synchro.php (the only difference is the language (french) "donnees_utilisateur" stands for "mydata") As you can see, the result is presented in clear text. Why can't I get it when the function is integrated in the larger script is a mystery... I made multiple verifications and scripts are good on both sides (Flash AS and PHP)... PS: About the article, I think I use the fair language for a newby. If not, simply ignore my postings... :-) Quote Link to comment Share on other sites More sharing options...
Germaris Posted December 13, 2012 Author Share Posted December 13, 2012 You should never mess with a user's password. A space is perfectly valid within a password. However, what OP is doing is removing any lead and trail spaces from the user's password. Basically, you're tampering with the user's password which will cause them to not be able to login next time around. echo md5(' '); // 7215ee9c7d9dc229d2921a40e899ec5f Always give warning to your user's, during registration, as to what you allow/require them to enter when creating their passwords. But don't be stingy and not allow them to use special characters or anything of the like. So, remove the trim() and hash the password straight up: $pass = md5($pass); *That's all I have until you explain exactly what it is you need help with, as Christian F. pointed out. Thanks for replying mrMarcus! During registration, I already warned that only Uppercases, Lowercases and Number can be used and a verification process insure this. However, as it is possible for the user to mistakingly enter a space when typing the credentials, I prefer to trim them. (and it's not messing with them...) For more on my problem please read my reply to Christian... Quote Link to comment Share on other sites More sharing options...
mrMarcus Posted December 13, 2012 Share Posted December 13, 2012 However, as it is possible for the user to mistakingly enter a space when typing the credentials, I prefer to trim them. (and it's not messing with them...) But it is still messing with their password. Say I enter (where <SPACE> is a space): HueLsdwy*823h<SPACE> I click "Register" and the form goes through successfully. Now, in the backend you've just trimmed that trailing <SPACE> without my knowing. The next time I go to login my trailing space is no longer valid and I will have no idea why. If your processing script recognizes an illegal character within the password input, do not continue. Notify the user that an illegal character was found and to try again. This renders the use of trim(), as you're using it, useless. Same goes for using mysql_real_escape_string() on a password before hashing it. If somebody were to use quotes within their password, mysql_real_escape_string() would escape those quotes with a \ which would change the user's hashed password. Just an FYI. Quote Link to comment Share on other sites More sharing options...
mrMarcus Posted December 13, 2012 Share Posted December 13, 2012 Thanks for your interest in my concerns. I simply have no result at all when using this function inside a very larger script grouping all the functions I use to make a Flash SWF to communicate with a MySQL Database. I made a simple script for testing purpose using the same function I posted here and... it works! You can have a look at the result at: http://www.notre-annuaire.com/strict/try_synchro.php (the only difference is the language (french) "donnees_utilisateur" stands for "mydata") As you can see, the result is presented in clear text. Why can't I get it when the function is integrated in the larger script is a mystery... I made multiple verifications and scripts are good on both sides (Flash AS and PHP)... PS: About the article, I think I use the fair language for a newby. If not, simply ignore my postings... :-) Are you sure that function is being referenced properly? Is your "larger scale" script reaching the point where that function is being called? Add some logging to the function so you can determine if it's being reached. And log any mysql_error()'s that may be occurring. Quote Link to comment Share on other sites More sharing options...
Germaris Posted December 13, 2012 Author Share Posted December 13, 2012 But it is still messing with their password. Say I enter (where <SPACE> is a space): HueLsdwy*823h<SPACE> I click "Register" and the form goes through successfully. Now, in the backend you've just trimmed that trailing <SPACE> without my knowing. The next time I go to login my trailing space is no longer valid and I will have no idea why. If your processing script recognizes an illegal character within the password input, do not continue. Notify the user that an illegal character was found and to try again. This renders the use of trim(), as you're using it, useless. Same goes for using mysql_real_escape_string() on a password before hashing it. If somebody were to use quotes within their password, mysql_real_escape_string() would escape those quotes with a \ which would change the user's hashed password. Just an FYI. You are wrong. Only uppercases, lowercases and numbers are allowed. And my form verification reject any user name or password containing anything else (including spaces) More than 4700 users have registered through almost nine years and no one has ever complained of a rejected credential... Quote Link to comment Share on other sites More sharing options...
Germaris Posted December 13, 2012 Author Share Posted December 13, 2012 (edited) Are you sure that (1)function is being referenced properly? Is your "larger scale" script reaching the point where that function is being called? Add some logging to the function so you can determine if it's being reached. (2)And log any mysql_error()'s that may be occurring. (1)Yes it is. (2)Already done since my first post and no error reported. Mystery... Edited December 13, 2012 by Germaris Quote Link to comment Share on other sites More sharing options...
mrMarcus Posted December 13, 2012 Share Posted December 13, 2012 You are wrong. Only uppercases, lowercases and numbers are allowed. And my form verification reject any user name or password containing anything else (including spaces) More than 4700 users have registered through almost nine years and no one has ever complained of a rejected credential... Then why are you using trim() on your passwords upon hashing if any user who enters a preceding and/or trailing space won't reach that point anyway? According to what you're saying, you should be able to remove trim() and all will be fine. Quote Link to comment Share on other sites More sharing options...
mrMarcus Posted December 13, 2012 Share Posted December 13, 2012 (1)Yes it is. (2)Already done since my first post and no error reported. Mystery... Can you provide the block of code which calls the synchrouser() function, please? Is the declaration of the synchrouser() function reachable/in scope of the script which is calling it? Is it declared within an included file that isn't resolving or isn't included properly? Have you correctly opened a connection to the database? Quote Link to comment Share on other sites More sharing options...
Germaris Posted December 13, 2012 Author Share Posted December 13, 2012 Then why are you using trim() on your passwords upon hashing if any user who enters a preceding and/or trailing space won't reach that point anyway? According to what you're saying, you should be able to remove trim() and all will be fine. You are right. I thought first to add to security... But it's true, "trim" is useless. Quote Link to comment Share on other sites More sharing options...
mrMarcus Posted December 13, 2012 Share Posted December 13, 2012 You are right. I thought first to add to security... But it's true, "trim" is useless. In this case, it is. Before hashing the password, you should ensure that the password in question meets all of your acceptable criteria. Check for spaces, special characters, etc., and if found, do not process any further (do not hash the password), and notify the user of their mistake. Once they have corrected the issue, they can try again. Once successful, you can then hash the password. I was under the impression you were continuing the processing even if a user had entered illegal characters and just modifying it for them on-the-fly, which is no good for obvious reasons. Just simply telling them ("Please do not use special characters and/or spaces") during registration is not enough as people don't usually care to read that stuff. Always validate user input. Quote Link to comment Share on other sites More sharing options...
Jessica Posted December 13, 2012 Share Posted December 13, 2012 As long as you mess with it the same way each time who cares. Quote Link to comment Share on other sites More sharing options...
mrMarcus Posted December 13, 2012 Share Posted December 13, 2012 (edited) As long as you mess with it the same way each time who cares. You shouldn't be messing with a user's password to begin with. If the user wants to enter 20 trailing spaces, let them. 50 even. It gets hashed to 32 char anyway, so length isn't an issue (Edit: of course you should cap the length at some point to avoid any malicious behaviour). I can't stand it when a site tells me how to enter my password. I know what a secure password is, and it involves special characters, including spaces. Once those are taken out of the equation, suddenly I don't feel as secure. Bad practice to have to "mess" with a user's input during registration and then again at login, especially when it's completely avoidable. Edited December 13, 2012 by mrMarcus Quote Link to comment Share on other sites More sharing options...
Jessica Posted December 13, 2012 Share Posted December 13, 2012 (edited) I agree 100%. But saying that using trim will make the site not work isn't true - as long as you trim it every time. Not to mention, that a simple phrase is way more secure than just "random" characters. You can make a much more secure password using only a-z than using special chars - what matters is the length. You said "I click "Register" and the form goes through successfully. Now, in the backend you've just trimmed that trailing <SPACE> without my knowing. The next time I go to login my trailing space is no longer valid and I will have no idea why." That is not true, as long as they trim it each time. That was all. Edited December 13, 2012 by Jessica Quote Link to comment Share on other sites More sharing options...
mrMarcus Posted December 14, 2012 Share Posted December 14, 2012 Not to mention, that a simple phrase is way more secure than just "random" characters. You can make a much more secure password using only a-z than using special chars - what matters is the length. Well yes, length does play a factor. But a combination of [A-Za-z0-9`~!@#$%^&*()_-+=}]{[|\"':;?/>.<,] will trump simply using [a-z]. Some cases, even if [a-z] is longer in length. Quote Link to comment Share on other sites More sharing options...
Jessica Posted December 14, 2012 Share Posted December 14, 2012 Not if you can't even remember it, and write it down. Quote Link to comment Share on other sites More sharing options...
Barand Posted December 14, 2012 Share Posted December 14, 2012 PMFJI "abcd<SPACE>" is more secure than "abcd". If you are going to trim the space off the end then anyone can log in with "abcd" instead of the correct "abcd<SPACE>" Quote Link to comment Share on other sites More sharing options...
Recommended Posts
Join the conversation
You can post now and register later. If you have an account, sign in now to post with your account.