Jacques1

PHP basics -- The foreach loop Or simply use array_filter().

What is the JSON document used for? In general, JSON documents carry raw data, so you'd use the raw, unformatted date.

No, no, no. You do not use numbered variables or names at all. This is very, very poor design. You want a list instead. Unfortunately, PHP is rather quirky when it comes to processing multiple form parameters of the same kind. You need a special syntax for this: <input type="text" name="foo[]"> Note the brackets. This turns $_POST['foo'] into an array.

Formatting is a presentational task, so it should be done in the template when the output is generated. It's a bad idea to format the date at database level or in the controller: What if you need to actually do something with the date in the application? Then the custom format will be in the way. What if you decide to support multiple formats? Do you put a big CASE block into every single query? Do you write a custom SQL function only to format dates? Both will be a pain in the ass. You might also have to pass a format parameter to the database system so that it knows which format to choose.

Sorry, but the suggestions that have been made so far are rather naïve and technically flawed. There are four problems: You're working around symptoms instead of fixing the actual problem (weak passwords). Anybody can lock out any user simply by entering a bunch of wrong passwords. This makes your website a perfect target for a denial-of-service attack. You're completely unprotected against an attacker trying a single password on multiple accounts (instead of multiple passwords on one account). Your counter mechanism (and Psycho's as well) can easily be circumvented with parallel requests: In your script, there's a time gap between checking the current counter and incrementing the value. Within that time frame, I can make as many attempts as I want, because the script will still “see” the old value. First of all, it's questionable whether a log-in limit even makes sense. If the passwords of your users are so incredibly poor that they won't even survive a couple of guesses, then that's the problem. You may be able to protect your log-in form, but once an attacker has direct access to your database (through an SQL injection, for example), you're screwed. This is actually very likely given that you didn't know anything about proper database code up until now. What you need are better passwords: Tell your users about password managers like KeePass, implement a password strength meter, maybe enforce a certain password policy. Help your users choose better passwords, then brute-force attacks won't be such a big problem. If you absolutely must have a limit, this will be complicated. It's not just a simple database query and a bunch of checks. First, incrementing the counter and getting the new value must be a single atomic operation so that there's no gap. One possibility is to use the LAST_INSERT_ID() function within an UPDATE query to update the value and capture it at the same time: <?php define('MAX_LOGIN_ATTEMPTS', 3); $database = new PDO('mysql:host=localhost;dbname=YOUR_DB', 'YOUR_USER', 'YOUR_PASSWORD', array( PDO::ATTR_EMULATE_PREPARES => false, PDO::ATTR_ERRMODE => PDO::ERRMODE_EXCEPTION, PDO::ATTR_DEFAULT_FETCH_MODE => PDO::FETCH_ASSOC, )); // increment the counter and capture the new value at the same time $login_counter_stmt = $database->prepare(' UPDATE users SET login_attempts = LAST_INSERT_ID(login_attempts + 1) WHERE name = :name '); $login_counter_stmt->execute(array( 'name' => $_POST['name'], )); // does this user even exist? if ($login_counter_stmt->rowCount()) { // get captured value $current_login_attempt = $database->query('SELECT LAST_INSERT_ID()')->fetchColumn(); // check the value if ($current_login_attempt <= MAX_LOGIN_ATTEMPTS) { // check password etc. } else { echo 'Maximum number of log-in attempts exceeded.'; } } else { echo 'This user does not exist.'; } Then you also need a global counter to prevent parallel attacks. If there's an unusually high number of failed log-in attempts, you need to take action. What exactly “unusually high” means depends on your system and needs to be decided by you. Last but not least, you need to think about denial-of-service attacks. Automatically locking accounts is generally bad idea and can make your site completely unusable. Instead, you should implement a throttling mechanism. A simple solution is a good CAPTCHA like Google's reCAPTCHA. Of course you might also implement a time-based throttle, but this is easier said than done. You can't just do something like “three attempts in five minutes”, because then again an attacker can block the legitimate user.

For Windows, I recommend KeePass. It's established, open-source and, most importantly, keeps your passwords on your own PC rather than somewhere “in the cloud”.

The problem is that you don't understand how prepared statements work. The whole point of a prepared statement is to not insert the values directly into the query string. This causes the usual SQL injection vulnerabilities and bugs. Instead, we use parameters to keep the values separate from the actual query. So you first send a query template with certain placeholders to the database system: $stmt = $this->db->mysqli->prepare(' UPDATE entry SET string = ?, delimiters = ? WHERE entryid = ? '); After this template has been processed by the database system, you can bind concrete values to the parameters and execute the prepared statement: $stmt->bind_param('ssi', $e->string, $e->delimiter, $e->id); $stmt->execute(); Again: The whole point is to keep the values separate from the query itself so that they cannot interfere with each other. If you just stuff everything into one big string, then the database system doesn't know which parts belong to your query and which parts are the user-provided values. It will simply execute the entire string. The try statement is also nonsense and downright dangerous. Printing the error message on the screen will expose internal data to the whole world. At the same time you lose important information like the exact location and a stack trace of the error. The default behaviour of exceptions is much smarter: They will send all information to an appropriate target like a log file (this can be configured in the php.ini). So leave the exception alone and let it do its job.

Your $valon variable is only defined if the condition of the if statement is fulfilled, but you use the variable in any case. That obviously makes no sense.

The code you've posted has trailing whitespace all over the place, which breaks the heredoc syntax. You need to make your editor/IDE remove excess whitespace, preferrably each time the file is saved.

Yes, this is safe. You should avoid plain for ... in loops, though, because they include inherited properties. If you or some library you've included happens to extend Object, then those extended properties will suddenly appear the the loop, even though you just wanted the properties from the JSON object. Use jQuery's $.each() instead or the classical hasOwnProperty() workaround.

There's no need for a full-blown JS template engine unless you actually create lots and lots of dynamic HTML with JavaScript. For a bunch of dynamic elements, it's overkill. And, no, Handlebars.js doesn't automatically apply URL-encoding. It's not that smart (it's actually not context-aware at all).

Yes, this is an XSS vulnerability. It has the same effect as dumping raw PHP values into HTML markup. The only difference is that your injection happens in the browser rather than on the server, which is called DOM-based XSS. So you must not inject raw values into any context. In the case of JavaScript, there are two secure alternatives: You generate a blank a element and then set the attributes and the content as text via attr() and text(). This is by far the most robust and foolproof solution. You manually HTML-escape the values. Neither JavaScript nor jQuery have a function for this, so you'd have to implement it yourself. Note that you also need to URL-encode the URL parameters.

An empty form field does have a value, namely the empty string. So the isset() check always succeeds unless you manually remove the form field so that the parameter isn't present at all. If you want to check for empty fields, you need the empty() statement. This covers both missing parameters and empty parameters.

A typical testing tool is PHPUnit which allows you write so-called unit tests for specific functionalities. However, unit tests only make sense if your code is modular. It must be possible to isolate each feature and run it in a controlled environment (external resources like databases can be simulated with dummy objects). If your code is just a bunch of big procedural scripts, you'll have a hard time implementing automated tests. Pretty much the only thing you can do is send HTTP requests to your scripts (with cURL, for example) and check if the database contains the expected values afterwards. While this is still better than nothing, it's of course much less precise than unit tests. You basically just get a “yes”/“no” feedback for the whole script. Which part does what is completely unclear. Either way, writing automated tests for complex features is not trivial. It takes a lot of time and careful planning. It's also questionable whether this time is a good investment. Almost all log-in scripts I've seen so far suffered from fundamental security issues rather than actual bugs. So if the choice is between spending one hour on OWASP to learn more about hash algorithms, and spending one hour on a couple of unit tests, I'd definitely pick the former.

He has clearly stated what he wants to do: secure the input. And validation does not provide security at all. It's a usability feature.

The first step is to stop using nonsense terms like “sterilizing”. Data is not “dirty”, so it cannot be “sterilized”. By itself, data doesn't do anything. The question is what you do with it. Do you want to insert the data into an SQL query? An HTML document? A JavaScript context? A PDF? Each case requires an entirely different security strategy. So any attempt of coming up with some magical universal “filter” is futile and conceptually wrong. You need to choose an appropriate solution for the specific context. For SQL queries, you either use prepared statements or manually SQL-escape the data. For an HTML context, you need HTML-escaping. To pass data to JavaScript, you use Ajax. Like I said, there is no one-size-fits all solution.

This is not the place where you just lean back and let others do your homework. You do the work, we just help. So what's the pattern? What have you tried? What are your thoughts?

No, it's not a good idea. It's actually plain wrong. What you need to understand is that the parameters of a prepared statement are treated as data. They're not processed by the SQL parser (that's the whole point). So if you have backslashes in your string, then you end up with literal backslashes. And that will certainly break your search function. You either use a prepared statement, or you manually escape the values. But you can't have both.

Just do a redirect, for heaven's sake. This “token” stuff is nonsense, at least the implementation that has been suggested. When will developers finally realize that people use multiple tabs in their browser? If I submit the form in two different tabs and then reload the first tab, I again end up with a duplicate submission, because the application has forgotten the first token and lets me use it again. If at all, you need a nonce (a number used only once). But this is much more complex than storing some uniqid() string in the session.

You know that you've said something stupid when jazzman1 steps in to defend you. I'll explain it one last time. If you still don't get it, just move along and take care of some other topic which you do understand. You said that it always depends on the context, which is one of those blanket statements people like to pull out of their arse when they haven't really thought about a problem yet. To prove your “point”, you refer to use cases like the search-and-replace function of a text editor. What you haven't taken into account (or maybe you don't even know it) is that standard regex engines which are implemented in editors, egrep etc. aren't even capable of processing nested expressions. This requires recursive subpatterns which are a special extension of some full-blown programming languages. In other words: In the cases where a regex might be valid, it's not powerful enough. And in the cases where it is powerful enough, there are better alternatives. So it's the wrong tool either way – which is exactly what I'm saying the whole time. I know that it's hard for programmers to understand the limitations of regexes. Many people don't even understand the theoretical background, and the people who do have a basic understanding of theory tend to miss everything else. It takes a lot of time to reach the next level where you're actually able to think about your tools and realize when they're not appropriate. Obviously you aren't there yet.

grep supports recursive subpatterns? Editors support recursive subpatterns? That was a lame trolling attempt, even by your low standards. Next month, try to come up with something intelligent. Otherwise you'll lose the last bit of respect that you may still have.

I find the whole approach rather weird. Where does this strange PHP object come from? The jQuery validation plugin is written specifically for JavaScript. You cannot store the rules as JSON, because JSON is a general-purpose data format which isn't bound to one particular language. Sure, you could use hacks like embedding JavaScript code within strings (which you appearently do). But that's a very poor solution. You'll need three rulesets: simple rules that can be shared between JavaScript and PHP (for example: whether a field is required, the minimum length, the maximum length) JavaScript-specific rules (callbacks, remote validators) PHP-specific rules (validation functions written in PHP) The shared rules are language-independent, so you can store them as JSON, XML or whatever. The language-specific rules need to be added within the particular language. For example, in JavaScript you would load the shared rules with Ajax and then add the callbacks, remote validators etc.

There are various techniques to implement this. The easiest way is to update the page every n seconds using Ajax. For example, every 5 seconds you would fetch the current text from the server and insert it into the page. It's also possible to actively push the new content to the client, but this is harder to implement and comes with various issues, so it's not recommended unless you specifically need “real time” updates.

None of this makes any sense whatsoever, and cryptography is definitely not the right place for guessing and trial-and-error. First of all, you said the private key is passed via the URL. You mean passed to the server? In that case you clearly don't understand how asymmetric cryptography works. The whole point of the private key is that it's indeed private. It isn't passed anywhere. If you send it to the server, then the entire concept is bogus. To be honest, the situation looks fishy: So some layman in your team(?) has invented a homegrown security protocol, and now they're asking another layman (you) to play around a bit and maybe come up with an implementation? What is this? A school project about bad cryptography?

Did you see that I answered your question? Or are you just here to whine and complain?