Jacques1

No, this is not secure. You can and should use parameters for the LIMIT clause, because that's the only way to make sure you'll never get an SQL injection. Don't just rely on $from_record_num and $records_per_page being "safe". If you get an error for the LIMIT clause, that's because haven't activated prepared statements (PDO::ATTR_EMULATE_PREPARES is true). You need to fix this: /* * Set up the database connection * - the character encoding *must* be defined in the DSN string, not with a SET NAMES query * - be sure to turn off "emulated prepared statements" to prevent SQL injection attacks and enable parameters in the LIMIT clause * - turn on exceptions so that you don't have to manually check for errors */ $dSN = 'mysql:host='.DB_HOST.';dbname='.DB_NAME.';charset='.DB_CHARSET; $databaseConnection = new PDO($dSN, DB_USER, DB_PASSWORD, [ PDO::ATTR_EMULATE_PREPARES => false, // activate prepared statements PDO::ATTR_ERRMODE => PDO::ERRMODE_EXCEPTION, // activate exceptions PDO::ATTR_DEFAULT_FETCH_MODE => PDO::FETCH_ASSOC, // fetch associative arrays by default (this isn't critical) ]); Now you can use parameters: // Example: create a prepared statement with two parameters $exampleQuery = $databaseConnection->prepare(' SELECT * FROM test LIMIT :offset, :limit '); // Execute statement and pass values to the parameters $exampleQuery->execute([ 'offset' => 0, 'limit' => 1 ]);

I understand, but still none of this is a good reason for duplicating the same try statement over and over again as opposed to using a single exception handler. What you describe is clearly a global error handling strategy, so it should be installed globally and not for every single query in your code. This also gives you much more flexibility. Maybe one day the errors will happen less in the database and more in some other area (networking, file handling, whatever). In your model, you'd have to rewrite the entire application, add new try statements everywhere and possibly remove the PDO-specific statements. With a global handler, you just have to change a few lines in a function.

Like I already said: If you have specific requirements, set up a custom exception handler on top of the standard error mechanism, preferrably in a prepended script. In that handler, you can do anything you like: Send e-mails, write the data to a special log etc. There are also premade solutions.

Just like the standard error handler! In other words, you don't need any of this. That's the whole point. Why is this so hard to understand? PHP already knows how to handle errors, because that's one of the most basic features of a programming language. It knows how to print stack traces, display the message, figure out the location of the error etc. There's absolutely no need for us to do it manually. Maybe your PHP environment is just horribly misconfigured. What happens when you throw an error without catching it? If you only get a blank screen, it's a problem of your configuration: Turn error_reporting all the way up, turn on display_errors, and now you should see a full error report with all relevant information. No extra work required.

Congratulations, you've just reinvented the standard error handler, except that your implementation sucks. The standard error handler requires zero extra code, it aborts the script in a controlled manner, and it can log the errors in production so that you have a chance to fix remaining bugs. Your implementation forces you to clutter the code with try statements, it fails to stop the script so that it will crash uncontrolled, and you don't log anything. So it's a lot of code for half as much features and twice as much problems. That doesn't sound like a good idea. Besides that, why are you so obsessed with PDO exceptions in particular? That's just a small subset of all possible errors. According to your logic, you'd have to put pretty much every single statement into a try block: <?php try { do_this(); } catch (SomeException $e) { include_once './config/catch_error.php'; } try { do_that(); } catch (SomeException $e) { include_once './config/catch_error.php'; } try { another_action(); } catch (SomeException $e) { include_once './config/catch_error.php'; } try { yet_another_action(); } catch (SomeException $e) { include_once './config/catch_error.php'; } And that helps us how exactly? I repeat: Just let the standard error handler do its job. It's much better than that, and it's already built into PHP. If you do need a special error feature (which you appearently don't), then install your own handler rather than drowning the code in try statements.

So what is the problem? Are you getting an error message? An unexpected result? “Doesn't work” doesn't really tell us anything. You need to get rid of the try statement, though. It's completely useless, it will suppress very important debug information (line number, stack trace etc.), and if you forget to remove it in production, your users will be greeted with weird error messages. Just leave the exception alone so that PHP uses the standard error handler.

Just use shuffle() in your application. There's no reason for doing this in MySQL. hansford's query is incorrect, because it yields 15 random rows which are then ordered by date (the selected fields also don't match). You want the reverse order of operations. But then again, why use MySQL in the first place?

This can be done with a single query if you fix your data model. Fields in an SQL table are not supposed to hold “arrays”, comma-separated values or anything like that. One field is for one value. If you need a many-to-many relationship between players and tanks, you create a third table which assigns player IDs to tank IDs. So your three tables should look like this: player - player_id - name - ... tanks - tank_id - name - ... player_tanks - player_id - tank_id - ... Now your data actually becomes accessible to your database system. To get a full matrix of all player/tank combinations, you use a CROSS JOIN between tanks and players (the cartesian product of both tables). Then you do a LEFT JOIN between the resulting table and player_tanks to figure out which combinations are actually in use. The tables and test data: CREATE TABLE players ( player_id INT AUTO_INCREMENT PRIMARY KEY, username VARCHAR(255) NOT NULL UNIQUE ); CREATE TABLE tanks ( tank_id INT AUTO_INCREMENT PRIMARY KEY, tank_name VARCHAR(255) NOT NULL UNIQUE ); CREATE TABLE player_tanks ( player_id INT NOT NULL, tank_id INT NOT NULL, PRIMARY KEY (player_id, tank_id), FOREIGN KEY (player_id) REFERENCES players (player_id), FOREIGN KEY (tank_id) REFERENCES tanks (tank_id) ); -- test data INSERT INTO players (username) VALUES ('Player A'), ('Player B'), ('Player C') ; INSERT INTO tanks (tank_name) VALUES ('Tank A'), ('Tank B'), ('Tank C') ; INSERT INTO player_tanks (player_id, tank_id) VALUES (1, 2), (1, 3), (2, 1) ; The query: SELECT tanks.tank_id, tanks.tank_name, players.player_id, players.username, player_tanks.player_id IS NOT NULL AS combination_exists FROM players CROSS JOIN tanks LEFT JOIN player_tanks ON players.player_id = player_tanks.player_id AND tanks.tank_id = player_tanks.tank_id ORDER BY tanks.tank_name ASC, players.username ASC ; Turning this into an HTML table is pretty straightforward: You iterate over the SQL rows, emit a table cell for each one, and whenever you encounter a new tank, you start a new HTML row.

The SELECT keyword in your first query is misplaced. Either leave it out entirely (just write down the function call expression) or enclose the subquery in parentheses. So it's either SPLIT_STRING(...) or (SELECT SPLIT_STRING(...))

There's no particular security problem with explode(). You have to treat the tags like any other dynamic value, that is, you have to escape them before showing them, you mustn't use them in a dangerous context like a script element etc. Your implementation sounds odd, though. Stuffing comma-separated values into a relational database is generally a bad idea, but it's particularly bad when you actually want to do something with the data. The whole point of tags is to allow tag-based searches, but that's nearly impossible when all tags are buried somewhere in large strings. Do you want to load the entire database into your application, step through each row, unpack the values and check if one of the tags match? That will obviously kill performance. Or do you want to try some LIKE magic? That's extremely cumbersome, error-prone and, again, inefficient. In the relational model, one piece of information goes into one row. For example, make one table for the tags, one table for the tagged objects (e. g. articles) and one table which assigns the tags to the objects (e. g. article_tags). Now searching by tag is trivial.

Simply marking rows as deleted is obviously much easier to implement and allows you to get rid of the extra table. However, there are two problems you need to be aware of: Whenever you query the table, you have to remember to exclude deleted rows. This can be fixed with a view which automatically filters the rows. Hidden rows can lead to a lot of confusion when they cause constraint violations. For example, let's say one of the columns is UNIQUE. When a user deletes the rows, they obviously think they can just reuse the value. But instead the database system complains about a conflict with some phantom row which shouldn't even exist (from the user's perspective). Your implementation also sounds weird. Why are you using strings? This is very confusing and error-prone. You should be using a BOOLEAN (this is actually an integer type, but MySQL knows how to interpret the values correctly). Use FALSE as the default value and TRUE to delete rows.

This is better than the original code, but it still has a lot of issues. Relying on “trusted” values is very risky and has lead to many security vulnerabilities in the past. A much better approach is to pass all input to a prepared statement and not take any chances. SQL identifiers (table names, field names etc.) also must be whitelisted, possibly even escaped and quoted. That's not only a security measure, it's also a matter of robustness. Many people use nonstandard identifiers which will crash the query when there's no appropriate quoting. Never show internal database errors to the user. That's simply none of their business, and you may leak critical information to attackers. Don't catch exceptions unless you know exactly what you're doing. Do not use SET NAMES as I already said above. This can be used for SQL injection attacks in certain scenarios. In fact, using highly dynamic queries where even the identifiers aren't predefined is stretching the limits of all standard database interfaces. That's simply not what they're made for. If you need this all the time, consider switching to a query builder. Or simply avoid those queries and use hard-coded identifiers. The function you've posted isn't really that useful, anyway. As to PDO and MySQLi in general, you'll want something like this: PDO <?php const DB_HOST = 'localhost'; const DB_USER = '...'; const DB_PASSWORD = '...'; const DB_NAME = '...'; const DB_CHARSET = 'UTF8'; /* * Set up the database connection * - the character encoding *must* be defined in the DSN string, not with a SET NAMES query * - be sure to turn off "emulated prepared statements" to prevent SQL injection attacks * - turn on exceptions so that you don't have to manually check for errors */ $dSN = 'mysql:host='.DB_HOST.';dbname='.DB_NAME.';charset='.DB_CHARSET; $databaseConnection = new PDO($dSN, DB_USER, DB_PASSWORD, [ PDO::ATTR_EMULATE_PREPARES => false, // activate real prepared statements PDO::ATTR_ERRMODE => PDO::ERRMODE_EXCEPTION, // activate exceptions PDO::ATTR_DEFAULT_FETCH_MODE => PDO::FETCH_ASSOC, // fetch associative arrays by default (this isn't critical) ]); // Example: create a prepared statement with two parameters $exampleQuery = $databaseConnection->prepare(' SELECT :left_operand + :right_operand AS sum_result '); $leftOperand = 1; $rightOperand = 2; // Execute statement and pass values to the parameters $exampleQuery->execute([ 'left_operand' => $leftOperand, 'right_operand' => $rightOperand, ]); // Fetch result $exampleResult = $exampleQuery->fetch(); // Inspect result var_dump($exampleResult); MySQLi <?php const DB_HOST = 'localhost'; const DB_USER = '...'; const DB_PASSWORD = '...'; const DB_NAME = '...'; const DB_CHARSET = 'UTF8'; // Turn on exceptions so that you don't have to manually check for errors $mysqliDriver = new mysqli_driver(); $mysqliDriver->report_mode = MYSQLI_REPORT_ERROR | MYSQLI_REPORT_STRICT; $databaseConnection = new mysqli('localhost', DB_USER, DB_PASSWORD, DB_NAME); // Define the character encoding; do not use a SET NAMES query $databaseConnection->set_charset(DB_CHARSET); // Example: create a prepared statement with four parameters $exampleQuery = $databaseConnection->prepare(' SELECT ? + ? AS sum_result '); $leftOperand = 1; $rightOperand = 2; // Bind values to the parameters $exampleQuery->bind_param('ii', $leftOperand, $rightOperand); // Execute statement $exampleQuery->execute(); // Bind results to variables and fetch them $exampleQuery->bind_result($sumResult, $productResult); $exampleQuery->fetch(); // Inspect result var_dump($sumResult, $productResult); As you can see, MySQLi is incredibly verbose, and it's naturally MySQL-only. With PDO you get a universal database interface with a much nicer API.

No, I meant mysql_set_charset() as a fallback solution in case durfy has to stick to the old MySQL extension. If it's possible to modernize the code, I'd use PDO rather than MySQLi. It's much more convenient and not bound to MySQL in particular.

I'm sorry to tell you, durfy, but pretty much every single line of the code is wrong. The mysql_* functions are ancient and will be removed in the future. Your PHP interpreter should actually tell you that (unless you're suppressing all warnings). You can't just insert variables directly into query strings. What if there's a space in $rowname or $tablename? What if there's a single quote in $findervalue? What if somebody deliberately tries to manipulate the query through any of the input (aka SQL injection)? Each of this will either result in a syntax error or a full-blown security vulnerability. Even if you happen to escape the input at some earlier point, your use of SET NAMES may break the escape mechanism, because it changes the database connection encoding without notifying PHP. You're not doing any error checking at all, so the code will continue after a failed query and crash at a later point with a weird error message (which is exactly what happened). So you have quite a lot of work ahead of you. If there's any chance to modernize the code and switch to a contemporary database interface like PDO, do that. This also allows you to use prepared statements and avoid the risk of SQL injections. If you're stuck with the old code, you need to manually escape every single input and check the return value of every single mysql_ function call (false means there was an error). And replace the SET NAMES query with mysql_set_charset().

Why not do it in MySQL itself? You already know CURDATE(), and for date differences, there's DATEDIFF(). This has the additional benefit that you'll never run into time synchronization problems in case your PHP and your database server are running on two different machines.

The class looks good, but you musn't store the keys in the database, and you should only generate a single key for the entire application. In fact, the whole point of the key is that it's not in the database so that an attacker who has gained access to the data (e. g. through an SQL injection) won't be able to read it. If the key is sitting right next to the ciphertext, the whole encryption is pointless. So generate a single server key, put it into some PHP configuration file and encrypt all sensitive data with that key. This is the standard procedure in many frameworks. Generating a new key for each user serves no purpose and only makes things more complicated. You also need to fix your database code. Inserting raw input into queries will result in an SQL injection attack sooner or later, which is particularly bad when you're dealing with sensitive data. Use prepared statements and bind the input to the statement parameters. This will make sure they cannot cause any harm.

So you want case-sensitive string comparison? Then you need to use a case-sensitive collation like utf8_bin for the column or the entire table.

This was just pseudo-code. If you're using the OpenSSL extension, you encrypt data with openssl_encrypt() and decrypt it with openssl_decrypt(). <?php const ENCRYPTION_ALGORITHM = 'AES-128-CBC'; // from the configuration file $server_master_key_hex = '1b438d8bf995e4152d07fa9e9626e4ee'; // decode the master key; this is absolutely crucial, because OpenSSL expects a binary key $server_master_key = hex2bin($server_master_key_hex); // encryption $plaintext = 'Hallo world.'; $iv = openssl_random_pseudo_bytes(openssl_cipher_iv_length(ENCRYPTION_ALGORITHM)); $ciphertext = openssl_encrypt($plaintext, ENCRYPTION_ALGORITHM, $server_master_key, false, $iv); echo "Ciphertext: $ciphertext<br>"; // decryption $retrieved_plaintext = openssl_decrypt($ciphertext, ENCRYPTION_ALGORITHM, $server_master_key, false, $iv); echo "Plaintext: $retrieved_plaintext"; This still needs error checking. Yes. However, note that there are two bugs in your current code: When the output turns out to not be cryptographically secure, you call the function recursively but don't override the previous result. There's no recursion limit, so you may run into a segmentation fault or exceed the XDebug recursion limit. It's better to have a while loop with a fixed limit. <?php $max_attempts = 10; $attempts = 0; do { $random_bytes = openssl_random_pseudo_bytes(16, $crypto_strong); $attempts++; } while (!$crypto_strong && $attempts < $max_attempts);

Hi, this doesn't make a lot of sense to me, and there are many serious defects in the code. First of all, what are you trying to achieve? I see a lot of keys derived from keys derived from keys but no clear concept. Who should know the data? Do you want the server to have access to all plaintext? Then you can forget about all this super-complicated user key stuff and just use a single server key. Or do you want to create a scenario where even the server doesn't know the data until the user has provided their personal key? Then you obviously musn't store the user keys in your database. You have to ask the user for their password, run the plaintext password through a key derivation algorithm like PBKDF2 and then use that key for encrypting or decrypting the data. The keys aren't stored anywhere. User-provided keys probably don't make sense in your case, because you wouldn't be able to do anything with the ciphertext stored in your tables except for showing the data to the user right after they're logged in. This is only useful if the data is purely private like e-mails in a webmailer. So I'll assume you want a single master key. Note that you must not use ECB mode like you currently do, or else all users with the same age, name etc. will end up with the same ciphertext. Use a proper mode like CBC. This means you'll have to generate a random initialization vector (IV) before encryption and then store the IV next to the ciphertext. So your table will look something like this: name_iv CHAR(32) | name TEXT | age_iv CHAR(32) | age TEXT --------------------+--------------+---------------------+------------- (random hex string) | (ciphertext) | (random hex string) | (ciphertext) ... Encryption and decryption should happen in the application, not the database system. If you share the master key with MySQL, this increases the risk of leaking it (e. g. through the query log). The pseudo-code looks like this: Encryption: master_key := fetch master key from configuration file name := the plaintext username name_init_vector := generate 16 random bytes name_ciphertext := AES_CBC_ENCRYPT(name, name_init_vector, master_key) age := the plaintext age age_init_vector := generate 16 random bytes age_ciphertext := AES_CBC_ENCRYPT(age, age_init_vector, master_key) insert (age_init_vector, age_ciphertext, name_init_vector, name_ciphertext) into users Decryption: master_key := fetch master key from configuration file select age_init_vector, age_ciphertext, name_init_vector, name_ciphertext from users age := AES_CBC_DECRYPT(age_ciphertext, age_init_vector, master_key) name := AES_CBC_DECRYPT(name_ciphertext, name_init_vector, master_key) Be very careful not to confuse the encoded keys with the actual binary keys! Currently, you use the hexadecimal representation of the master key for encryption, which means each of the 16 bytes only has 4 bits set. This reduces the actual key length to only 64 bits. Do you follow me so far?

Don't stuff all validations into a single function. Make one function for each check, e. g. valid_email_address() for e-mail validation. Some checks are rather sloppy. For example, your “names” pattern permits strings consisting only of commas or whitespace, which is probably not what you want. A more reasonable pattern would be something like '/\\A[ \\t]*\\w+[ \\t]*(,[ \\t]*\\w+[ \\t]*)*\\z/' This checks for an actually comma-separated list of words, optionally surrounded by whitespace. Filenames shouldn't start with a dot, because this is interpreted as a hidden file. A single dot or double dots also have a special meaning. So I'd rather use '\\A\\w+([.]\\w+)*\\z' Why do you restrict the characters allowed in a password? This massively reduces the password strength for each given length, because there simply aren't many combinations to choose from. It's also extremely annoying for people who generate random passwords with a password manager, because they have to change their settings in order to comply with your policy. Passwords should have no restrictions at all or only very basic restrictions like “The word must be printable”. Artificially reducing the input alphabet is a bad idea.

You mustn't apply URL-encoding to the entire URL, only the parts which actually need to be encoded. Encoding means that certain characters which have a special meaning in a URL (like slashes, question marks etc.) are turned into literal characters. For example, if you wish to insert a dynamic segment into the path, you'd encode that to prevent it from changing the structure of the URL: $link = 'https://mysite.com/users/' . rawurlencode($username); Without encoding, the $username variable could manipulate the path by injecting slashes, or it could inject an entire query string.

Sorry, I mixed up the two headers. So, yes, it's DENY for the X-Frame-Options header, and frame-ancestors 'none' for CSP. CSP also allows you to virtually eliminate the risk of XSS attacks by blocking all scripts and style sheets except those coming from a trusted source. Ideally, you'd have a separate domain serving only static content: Content-Security-Policy: default-src https://static.yoursite.com; img-src * data:; connect-src 'self'; child-src 'self'; frame-src 'self'; frame-ancestors 'none' Note, however, that strict CSP has some implications: You need clean HTML markup without tons of inline scripts and stylesheets, because those are blocked as well. It's possible to whitelist specific inline scripts and styles by hashing their content, but legacy websites may be too difficult to clean up. Only modern browsers (and not IE) benefit from CSP, and not every browser supports every directive.

The X-Frame-Options header should be set to NONE. Do you even have a frame which includes one of your own pages? Then relax the policy for this specific page rather than globally. Using SAMEORIGIN is risky, because it can be used to defeat the clickjacking protection via nested frames. The X-Frame-Options only takes the top-level frame context into account, so if you have a frame containing an external site, then that site may frame your site despite the SAMEORIGIN restriction. Intermediate frame contexts aren't checked. A much better solution is Content Security Policy (CSP). It handles nested frames correctly and also provides advanced protection against cross-site scripting. The frame-ancestors directive of the CSP header is actually the successor of the X-Frame-Options header. Note, however, that CSP isn't supported by all browsers, so it's a good idea to use both headers at the same time.

It's very important to specify the character encoding of the HTML document when using htmlspecialchars(). Otherwise the function can become useless. You should also set the ENT_QUOTES flag so that both single and double quotes are escaped: E. g. htmlspecialchars($input, ENT_QUOTES | ENT_SUBSTITUTE | ENT_HTML5, 'UTF-8'); The ENT_SUBSTITUTE and ENT_HTML5 flags aren't critical. To avoid encoding the URL parameters by hand (which is somewhat messy and error-prone), you can also use http_build_query() <?php $link = 'https://example.com?' . http_build_query(['x' => 'foo', 'y' => 'bar']); ?> a href="<?= htmlspecialchars($link, ENT_QUOTES | ENT_SUBSTITUTE | ENT_HTML5, 'UTF-8'); ?>">Link</a>

Some additional suggestions: 1) PDO uses a potentially insecure default configuration which you should override. In particular, prepared statements are deactivated by default, so your prepare() doesn't prepare anything. It merely auto-escapes the input values and inserts them into the query string, which is known to cause vulnerabilities in some scenarios. PDO also doesn't use exceptions by default, so the script will ignore failed queries and crash uncontrolled at a later stage. A proper configuration looks something like this: $database_connection = new PDO(DATABASE_DSN, DATABASE_USERNAME, DATABASE_PASSWORD, [ PDO::ATTR_EMULATE_PREPARES => false, // use real prepared statements instead of "emulated" ones PDO::ATTR_ERRMODE => PDO::ERRMODE_EXCEPTION, // throw an exception in case of an error PDO::ATTR_DEFAULT_FETCH_MODE => PDO::FETCH_ASSOC, // fetch associative arrays by default (this is not critical) ]); 2) Do not mangle the user input with questionable PHP features like FILTER_SANITIZE_STRING. This is extremely confusing and even dangerous for the user, because the filter cuts off anything that looks like HTML. So a good password like “sfjk3<§45%$§%g” is silently turned into the weak password “sfjk3”. It's best to leave the user input alone (except maybe for trimming the username). Store the original data and escape it when needed. Speaking of which: Any data you wish to insert into an HTML context must first be escaped with htmlspecialchars() to prevent cross-site-scripting attacks. <?php function html_escape($input, $encoding) { return htmlspecialchars($input, ENT_QUOTES | ENT_SUBSTITUTE | ENT_HTML5, $encoding); } $username = 'foo'; ?> Welcome, <?= html_escape($username, 'UTF-8') ?>. 3) Your way of enforcing unique usernames doesn't work reliably. If two clients request the same unused name at the same time, the application allows both of them to have it, because the check happens before the rows are inserted. This will crash the application or lead to two different users with the same name, depending on whether the database system enforces unique values. A better solution is to let the database handle the uniqueness check: Make sure the username column is a primary key or has a UNIQUE constraint. Then simply try to insert the row with no prior database checks. If that causes a constraint violation, you know that the name is already taken. This approach is also much shorter than your current code, because you only need one query: <?php // MySQL error codes const MYSQL_ER_DUP_UNIQUE = '23000'; // ... database connection etc. $username = 'foo'; $email = 'foo@example.com'; $password_hash = '$2y$12$nGbRUyeeFfS/xr1uceoSz.mTRypyBR2rTi1gJL20mEhyXgAP3UvKS'; $member_query = $database_connection->prepare(' INSERT INTO members SET username = :username, email = :email, password = :password '); // Try to insert the new member; this will trigger an exception if the name is already taken. try { $member_query->execute([ 'username' => $username, 'email' => $email, 'password' => $password_hash, ]); } catch (PDOException $member_query_error) { // If the exception was caused by a constraint violation, tell the user about the duplicate name; otherwise propagate the exception. if ($member_query->errorCode() === MYSQL_ER_DUP_UNIQUE) { echo 'Sorry, this name is already taken.'; } else { throw $member_query_error; } } 4) Don't catch exceptions unless you know exactly why you're doing it. Internal database problems are none of the user's business, so there's no need to tell them. Even worse, you throw away all the error information and won't be able to analyze the problem. The default error handler is usually better than any custom solution, because it can be fine-tuned with the php.ini. If you want to have a user-friendly error message, just set up a custom error page for 500 errors (the manual of your webserver will tell you how). Catching exceptions only makes sense if you need to solve a specific problem in a special way (e. g. retry an action which is known to fail from time to time). This is very rare. Most of the time, you won't need any try statements at all. 5) Your use of password_hash() is not ideal. One of the main features of modern password hash algorithms is that they're adjustable. You can and should fine-tune them for your specific hardware and your security requirements. Sure, you can rely on the default configuration. But this means the hashing procedure is probably either too weak or too strong. It's better to explicitly choose an algorithm and set the parameters. The current standard algorithm is bcrypt which has a single strength parameter. You should decide how long the hashing should take (more time means more password security but also less usability) and then try different strengths until you end up with the right duration, e. g. one second for normal user accounts. <?php const PASS_ALGORITHM = PASSWORD_BCRYPT; const PASS_STRENGTH = 12; // increase or decrease as needed $password = 'testtest'; $hash = password_hash($password, PASS_ALGORITHM, ['cost' => PASS_STRENGTH]);