Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

**Warning** Your code has multiple security and usability issues. #1

Open
kamil-tekiela opened this issue Mar 11, 2019 · 5 comments
Open

Comments

@kamil-tekiela
Copy link

Your coding example has many serious issues, and yet it has come up for me in Google search as one of top results. Could you please either rewrite the code or take it offline, so that new PHP developers don't make the same mistakes?

  1. You are wide open to SQL Injections and should really use parameterized prepared statements instead of manually building your queries. They are provided by PDO or by MySQLi. Never trust any kind of input, especially that which comes from the client side. Even when your queries are executed only by trusted users, you are still in risk of corrupting your data.
  2. Never store passwords in clear text! Only store password hashes. Use PHP's password_hash() and password_verify() . If you're running a PHP version lower than 5.5 (which I really hope you aren't), you can use the password_compat library to get the same functionality.
  3. Use utf8mb4 for your DB encoding, rather than latin1. At this day and age users should be able to use full range unicode characters in their usernames.
  4. Don't strip off user's passwords before entering them into DB. In fact don't execute htmlspecialchars on the data being entered into DB. The whole purpose of this function is to sanitize data being displayed in HTML!
  5. base64_encode is not a hashing mechanism. It should never be used in connection with passwords! It makes no difference whether you do or not, because everyone knows either way that you have used 12345 in your example script.
  6. Don't kill your script with die() unless you really, really must! This should only be used if the rest of the script should not be executed, not as a control flow mechanism.
@keaikitse
Copy link

Im not having any error but its displaying this message "{"status":false,"message":"Username already exists!"}"

@Vedprakash19
Copy link

Your coding example has many serious issues, and yet it has come up for me in Google search as one of top results. Could you please either rewrite the code or take it offline, so that new PHP developers don't make the same mistakes?

  1. You are wide open to SQL Injections and should really use parameterized prepared statements instead of manually building your queries. They are provided by PDO or by MySQLi. Never trust any kind of input, especially that which comes from the client side. Even when your queries are executed only by trusted users, you are still in risk of corrupting your data.
  2. Never store passwords in clear text! Only store password hashes. Use PHP's password_hash() and password_verify() . If you're running a PHP version lower than 5.5 (which I really hope you aren't), you can use the password_compat library to get the same functionality.
  3. Use utf8mb4 for your DB encoding, rather than latin1. At this day and age users should be able to use full range unicode characters in their usernames.
  4. Don't strip off user's passwords before entering them into DB. In fact don't execute htmlspecialchars on the data being entered into DB. The whole purpose of this function is to sanitize data being displayed in HTML!
  5. base64_encode is not a hashing mechanism. It should never be used in connection with passwords! It makes no difference whether you do or not, because everyone knows either way that you have used 12345 in your example script.
  6. Don't kill your script with die() unless you really, really must! This should only be used if the rest of the script should not be executed, not as a control flow mechanism.

Try MD5 for storing password ( hash ) . It works in all php , mysql

@kamil-tekiela
Copy link
Author

@Vedprakash19 "Try MD5 for storing password ( hash )". No! Please do not use MD5 for hashing password.

@Vedprakash19
Copy link

Vedprakash19 commented Mar 27, 2020 via email

@kamil-tekiela
Copy link
Author

@Vedprakash19 Because it is not much better than plain text passwords. You can Google that https://www.google.com/search?q=md5+for+passwords&rlz=1C1CHBF_enIE798IE798&oq=md5+for+passwords&aqs=chrome..69i57.3047j0j1&sourceid=chrome&ie=UTF-8

Why would you want to use MD5 if you have proper PHP functions for password hashing: password_hash()

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants