Hacker News Reader

Comment by unshift | Hacker News Reader

nice to see people are still oblivious to sql injection and using string interpolation instead of prepared statements.

Good point. I should note that I'm not oblivious, but I wanted the example to be as readable as possible. I've added a note at the bottom of the post to make it clear that the code should be modified before using it in a live appplication.

Simple code examples with glaring security holes are a great way to teach people how to write insecure code. If you show usable code in a tutorial it will make it into a production environment somewhere.

Is that really a concern, though? People ignoring warnings like the one at the bottom of the post are probably going to write insecure code anyway. From a purely selfish standpoint, there is no advantage to me from not posting insecure code examples. In fact, the more people copy-paste such bad code and make sites that get hacked, the more opportunities I have for work when they get fired.

That really is pretty selfish.

I'm thinking there are two sides to this, the first is that just like you added the 'don't use this' as an after thought the majority of the people that find your code will cut and paste it without actually reading the article, the second is that if this is your 'first approach' to keep it readable you probably have at least a few instances where you forgot to update to more solid code at a later stage because you thought 'x' or 'y' is not facing the web at the moment. And then one day someone bridges two systems and bang, security hole.

I know it's selfish, I just thought it was a direction of thought worth exploring. Personally I wouldn't post insecure code without a lot more warnings, closer to (or commented in) the code itself. At the same time I would have absolutely no sympathy for a "programmer" that would copy paste code without even reading the entire blog post it's from, let alone make an effort to understand it. They don't deserve their job.

Your point on 'first approach' security holes accidentally being persisted is a good one, and I can certainly think of a few bits of code I wrote that were never meant to be secure, but could potentially be used in a larger, web-facing project at some point. Some food for thought there on perhaps never writing insecure code, even if it's just a test.

Tangential addendum: If security is Done Right, then there shouldn't be a choice between "easy to write, read and follow" and "secure".

After years of blogging, occasionally on programming-related topics, and years of managing an outsourcing operation, my guiding star for all code samples is "this will be used without modification by the least competent member of the cheapest team identified by a non-programmer working in the procurement department of the company which handles my mother's medical insurance."

If you have a blog where you routinely discuss topics that might come up in trivial back office code, you can probably think of a comment or email or hundred from the type of developer I'm thinking of.

I'd say that anyone who needs a tutorial to tell them how to do this in PHP probably doesn't have the skills to fix the security hole. You're setting people up to fail, which isn't very nice (although I'm sure you didn't intend it that way).

Seriously, it's worth making it a tiny bit less readable to makie it copy/paste safe!

If readability is your goal, the code should be focused on the task you're trying to teach. Code to put the result in a database is ancillary and could be cut, for a positive impact on readability.

There is a common misconception that secure code has to be by definition less readable and harder to write. The upvotes on the parent comment suggest that even HN readers think that way.

I believe that this is a false dichotomy. Good practices and separation of concerns often increase code readability. For example I think the updated version of your code with explicit parameter binding is much more readable than string concatenation.

It's clearly a proof of concept code focusing on the actual topic, and he begins the code with "Here's an example".

I take your point but unshift is still correct. One of the reasons PHP has a reputation for being insecure is because there are so many tutorials lying around that show the wrong way to do it. When newbies Google for solutions, they copy + paste what they find, they don't (bother | know that they should) look through it to see if there are any potential security holes.

A very important and effective technique in teaching others (regardless of the subject) is to never write or display something that's wrong, because it's often the only thing a student will remember. This is obvious when it comes to language, but it applies to pretty much anything.

Is that better or worse than not teaching anything because your student couldn't pick out the 10 lines of actual functionality among the 100 lines of paranoiaplate in each example?

Based on everyone's feedback, I've switched the example to the more-secure prepared statements.

Thanks, everyone!

Reply on news.ycombinator.com