Jacques1

It doesn't matter what you think happens. Where's your demonstration? I've posted mine. Just to make sure I understand you correctly: You're saying that PDO makes it impossible to reuse a prepared statement? That it somehow magically closes the statement, recreates it and also fakes the session statistic to confuse us? Um, yeah, that's an interesting theory. Where do you see that in the PDO code? I see exactly two occurences of mysql_stmt_prepare(): Once for PDO::query(), once for PDO::prepare(). https://github.com/php/php-src/search?q=dbh-%3Emethods-%3Epreparer&ref=cmdform Please point to the exact code that does what you think. Not sure if I am.

Yeah. I really have no idea how you think prepared statements work or what you're objecting against. The last post doesn't even make sense to me. The execute() method does exactly what it says: It executes the prepared statement. It does not create a new statement, and it does not destroy the current one. You can easily verify that yourself: <?php $database_options = array( PDO::ATTR_EMULATE_PREPARES => false , PDO::ATTR_ERRMODE => PDO::ERRMODE_EXCEPTION , PDO::ATTR_DEFAULT_FETCH_MODE => PDO::FETCH_ASSOC ); $database = new PDO('mysql:host=localhost;dbname=test;charset=utf8', 'root', '', $database_options); show_prepared_statements(); // *One* prepared statement $stmt = $database->prepare('SELECT 1 = :x'); // ... getting executed 10 times for ($i = 1; $i <= 10; $i++) { $stmt->execute(array('x' => $i)); } echo '<br>'; show_prepared_statements(); function show_prepared_statements() { global $database; $statements = $database->query("SHOW SESSION STATUS LIKE 'Com_stmt_%'"); foreach ($statements as $statement) { echo $statement['Variable_name'] . ': ' . $statement['Value'] . '<br>'; } } Besides the two SHOW SESSION queries, we have 1 prepared statement which gets executed 10 times. Just what we (or at least I) expected. The prepare()methods creates a prepared statement. And then you can execute it as often as you want using the execute() method. I don't know how else I could explain this. I've never met anybody who would doubt this simple mechanism.

I really wonder what you're reading into my code. I create the prepared statement before the loop. Then I execute it in the loop. So it's one prepared statement which gets executed multiple times. This isn't the first time I'm using PHP. And I repeat: Worrying about 23 simple INSERT queries is just silly. I don't understand why we're spending half of the discussion on this useless triviality. Should the OP ever experience performance issues, it's certainly not because of 23 queries.

Hi, I'm sorry to tell you, but this code is just awful from top to bottom. It's a walking security vulnerability and pretty much a collection of every mistake you can make in PHP. There's zero protection against SQL injections. Anybody can manipulate your queries and fetch any data they want, including all passwords. Even worse, the script stores the passwords as plaintext. Not only in the database, but also in the session files and a cookie. What – the – f*ck? Seriously, who would do this? Every schoolkid understands that passwords must be protected. I'm sure you could find plenty of other vulnerabilities if you look closer. The code is generally very, very poor with tons of obsolete stuff and bad practices. If you're serious about your server and your users, you must take this down now. I know, this sucks. But getting your server hacked and all passwords stolen sucks even more. And now that your code is public, I wouldn't be surprised if people actively start looking for your website as an easy target. Seriously, take this offline. I'm sure we can help you find a secure alternative.

This is an INSERT statement, not a SELECT statement. Your problem is fetching the rows, which obviously isn't necessary when doing an INSERT query.

Prepared statements don't work like this in MySQLi. Check the manual on how to do them: http://www.php.net/manual/en/mysqli-stmt.bind-result.php

I have no idea what you think is wrong. I suggest making one INSERT query per dataset. This should be done with a prepared statement: <?php // I assume we're using PDO $insert_something_stmt = $database->prepare(' INSERT INTO whatever SET user_id = :user_id, team_id = :team_id, points = :points '); // now the loop foreach ($_POST['something'] as $something) { $insert_something_stmt->execute(array( 'user_id' => $something['user_id'], 'team_id' => $something['team_id'], 'points' => $something['points'], )); } This is by the far the simplest and most secure solution. Another option would be to assemble one big INSERT query with multiple rows as suggested by Barand. In my opinion, this is a silly micro-optimization.

“Kill server performance” because of 23 trivial queries which he probably only does once in a while? C'mon. Micro-optimizations like this are just silly. What matters is security and simplicitly, and this can be achieved best by inserting the rows one by one with a prepared statement.

So that the whole world sees your MySQL issues and all the sensitive information associated with them? Internal error messages go into the log file, never on the user's screen. What you want is trigger_error().

Instead of numbering the values, put them into an array. PHP has a special syntax for this: <input name="whatever[0][user_id]"> <input name="whatever[0][team_id]"> <input name="whatever[0][points]"> <input name="whatever[1][user_id]"> <input name="whatever[1][team_id]"> <input name="whatever[1][points]"> etc. Then your $_POST array will be filled with nice subarrays consisting of a user_id, a team_id and points. Putting this into the database is trivial now: <?php foreach ($_POST['whatever'] as $whatever) { query(' INSERT INTO something SET user_id = ' . escape($whatever['user_id']) . ', team_id = ' . escape($whatever['team_id']) . ', points = ' . escape($whatever['points']) . ', '); } Yes, I do recommend making separate queries instead of messing with one big INSERT query.

Well, it simply makes no sense. How is $x better than $_POST['x']? Sure, it saves you 9 characters. But it clutters the global namespace and kills the security. If you absolutely definitely must import the variables, at least use extract() with the overwrite flag set to EXTR_SKIP.

Again: I doubt that. Given this gigantic security risk (in the admin area!), what makes you think the rest of your security is absolutely perfect? I don't even get the point of this weird variable import. The values are all in the $_POST array already. What's the point of stuffing them into separate variables?

Ugh. This is like the evil twin of the register_globals brainfart that PHP used to have in the dark ages until they finally fixed their security. Do you realize that this allows any user to set or overwrite any variable in the application? Given the code, I highly doubt that.

Hi, you have 72 numbered variables? Why? There's this thing called array, you know? Put the values into a proper array, then loop through the array and do one INSERT query (or better: a prepared statement) per dataset.

Not quite. You must stop the script after the redirect. Otherwise, it will happily keep running, which is a very common security vulnerability. Putting the session_start() into an if statement is also poor practice, because the session will be uninitalized if the condition isn't met. That means every attempt of accessing $_SESSION will throw a notice. Last but not least, get rid of the $_REQUEST variable and properly fetch the data from either the URL ($_GET) or the request body ($_POST).

Neither MD5 nor SHA-512 were ever intended to be password hash algorithms. They're primarily designed for digital signatures, which means they're very fast and take up very little resources so that they can be implemented on weak hardware like smartcards. Speed and low hardware requirements are pretty much the last thing you want when you're trying to protect passwords. Thanks to those properties, a stock PC can calculate billions(!) of MD5 hashes per second and millions of SHA-512 hashes. See this chart for concrete figures: oclHashcat – Performance It's easy to see what that means: Let's say a typical password has 8 characters from the alphabet a-zA-Z0-9. That's 64^8 = 281,474,976,710,656 possible passwords. According to the chart above, an HD 7970 calculates 8,089,000,000 MD5 hashes per second and 74,000,000 SHA-512 hashes. So trying out every single password in this range takes ~10 hours for MD5 and ~44 days for SHA-512. And that's just a naive brute-force attack with an average gamer PC. Actual attackers use sophisticated patterns and cloud computing services to gain much higher performance. Instead of hours and days, we're now talking about seconds and minutes. I hear people babbling about “rainbow tables” all the time. Who needs a rainbow table if they can simply calculate all hashes ad hoc? Long story short: SHA-512 and MD5 are dead (for password hashing). It doesn't matter which one you use. It doesn't matter if you salt those hashes. None of this makes a difference, because an attacker can simply grab some more dollars and make up for it. The only workaround would be to use extremely strong passwords (something like 16 bytes read from /dev/urandom). But that's obviously not a solution for the general public. If you want to protect passwords, you must use a specialized algorithm with three features: It must be computationally expensive The cost factor must be adjustable so that it can be increased as hardware becomes better It must generate a random salt per hash so that an attacker has to break each hash individually Possible choices are bcrypt and the newer scrypt. Since bcrypt is much better tested and supported, that's the way to go. See the link above for the PHP implementation. Anything other than bcrypt or a similar algorithm is unacceptable.

Two things: You still haven't replaced your code with Ch0cu3r's. You've merged his code into your while loop, which simply doesn't work. No while loop, no strpos(), just preg_replace_callback(). The syntax error means that you're running some old PHP version which doesn't support anonymous functions yet. You need a different approach then: <?php function replace_fraction($matches) { // replacement stuff goes here } $insert = preg_replace_callback("/([\d]+)\/([\d]+)/m", 'replace_fraction', $insert); The current check makes no sense, though. Dividing 0 is perfectly fine and simply yields 0. What you musn't do is divide by 0. So you only check the denominator, not the numerator. And this doesn't throw any error message. It silently replaces the invalid fractions with the string "division by zero". Not sure if that's what you want. I was actually hoping you would write the code yourself and learn from it instead of copypasting other peoples' solutions. But after Ch0cu3r has jumped in, I guess we can forget about that.

Hi, I hope you haven't used the scripts from your first two posts? Both are awful and not secure at all. Unfortunately, too many people run around writing “security tutorials” right after their very first “Hello world”. Writing a full review is too much, so I'll focus on the worst failures: SHA-512? Seriously? It's the 21st century. An average gamer PC easily calculates hundreds of millions of SHA-512 hashes per second. If you spend a few dollars on a cloud service, you can have many billions of hashes per second. It's easy to see that even strong passwords aren't gonna survive such an attack, no matter how often they've been salted. The only effective way of protecting passwords against brute-force attacks is to hash them with a specialized algorithm like bcrypt. PHP 5.5 supports it natively. Otherwise, there's a compatibility library with the same functions. In the script, there's a function which is supposed to stop people from doing more than 5 login attempts. Unfortunately, this is completely ineffective due to a naive implementation: They first check the current value, then they do all kids of stuff, and finally they increment the counter. Well, what if make 100 attempts before the counter is incremented? Then they will all be accepted, because the script still sees the old counter value. The same mistake is made in the registration script. They check if the name is already registered, and if it's not, they accept it. That means if two people request the same name at the same time, they may both be allowed to use it. Then they hash the password together with the user agent and store the result in the session to somehow check the login status. WTF? The password doesn't go anywhere except into the user table. You certainly don't store it in dozens of unencrypted session files loitering on the server! There are plenty of other brainfarts. For example, they run the user ID and the username through some homegrown filter before storing them in the session, because they “might print this value”. What? Have they never heard of escaping? Mistakes like this show a deep misunderstanding of programming and security fundamentals. That code is really the last thing you want to run on your server. You should generally be much more careful with your sources. Don't just copypaste code from some crappy amateur site. Make sure the people you're learning from actually know their stuff. You'll find them in big projects on Github, not so much on “wikiHow”.

When you use preg_replace() as suggested, there's no risk of running into an infinite loop. If you're asking how to deal with divisions by 0, well, that's up to you. You can reject the input altogether or replace the invalid fractions with error messages or whatever. That depends on what you wanna do with the numbers and on your audience. If your users get weird results for seemingly simple math operations, they will wonder what the hell is going on (unless they're familiar with IEEE floating point numbers).

Hi, that code is weird. Is there any reason why you are not simply using preg_replace()? Your current code has two problems: If there's a division by 0, you're not doing anything with the input. That means your loop runs forever. The second problem is that you keep overwriting $var (which is also a really crappy name for a variable). I suggest you use preg_replace() and fix your variable names. You also need to be aware that replacing fractions with floats will lead to “strange” results, because even simple fractions like 1/10 cannot be expressed as floating point numbers. This will be very confusing for the users.

Example for what? Getting your server hacked? You do not use multi-queries. At all. As I've just said, this is a gigantic security risk, because any injection vulnerability (like yours) allows the attacker to run arbitrary queries. If you want to do a dynamic query, you use a http://www.php.net/manual/en/mysqli.prepare.php'>prepared statement. And if you wanna repeat this query multiple times, you simple execute the statement multiple times. If you show us the real code, I'm sure we can help you with it.

No offense, but WTF are you doing there? This is like a collection of everything you must never do when writing a web script. So your PHP script has admin rights on the database. Then you fetch some data from an external site (through plain HTTP, I guess) and insert it straight into a multi query without any escaping or whitelisting whatsoever. Do you not understand the implications of this? This is like a mega-SQL-injection. Not only can an attacker manipulate existing queries. They can actually write their own queries and make use of the full permissions you've granted them. Vulnerabilities like this are commonly used to take over the whole server. If this is online, take it down immediately. The next problem is the weird database design. But security is much more important for now.

If the date field has a proper type (DATE, DATETIME or TIMESTAMP), you simply use the date and time functions: http://dev.mysql.com/doc/refman/5.6/en/date-and-time-functions.html If it's some hard-coded VARCHAR (looks like it), you have a problem. Since MySQL doesn't know what to do with your strings, you first have to repair your database by turning all pseudo-dates into actual dates. SELECT DATE_FORMAT(date, '%M') AS month, COUNT(*) AS name_count FROM test GROUP BY EXTRACT(MONTH FROM date) ORDER BY EXTRACT(MONTH FROM date) ASC ;

Hi, since you showed us the form and not the validation code, I have no idea how we're supposed to help you. However, your code has a bunch of serious security issues: First of all, it's probably vulnerable to cross-site scripting attacks through $_SERVER['PHP_SELF']. Depending on the configuration, Apache allows the user to append arbitrary pseudo-directories to the actual file path. For example, they could request your script like this: https://www.yourdomain.com/yourscript.php/<some JavaScript injection> Apache would accept it as a valid path for yourscript.php, and you would happily insert the JavaScript code into your markup. This again shows that you must escape any user-defined input before it can be inserted, no matter how restricted it may seem at first sight. While you do escape the $_POST values, you have totally forgotten to specify which character encoding should be used. This again can lead to cross-site scripting in some cases. If the default encoding of htmlspecialchars() simply isn't the one you actually use for your document, the escaping mechanism may fail to recognize the critical characters and let them through. For example, there's an infamous UTF-7 attack which takes advantage of an encoding mismatch. Last but not least, you must never send the password back to the client. Passwords are obviously very sensitive data, so the last thing you wanna do is send them back and forth around the globe. Apart from that, how exactly does this help the user? The passwords are masked, so the user can't just edit them. Wrapping it up: Never insert raw user input into your HTML markup. The request path is user-defined input. Escaping depends on the character encoding, so always specify the encoding when you use htmlspecialchars(). It has to match the charset attribute of the Content-Type header. If you do not have a Content-Type header with a charset attribute, add it now. Be very careful with passwords. Do not send them around.

Hi, I guess even the author of the code doesn't know what it's for. Where on earth does this come from? The code is an extremely cumbersome way of writing md5(time()) Why would one hash the current Unix time? I have absolutely no idea. Maybe they wanted a fixed length. Or maybe it's a botched security feature.