Back in ye olden days, almost every answer involving a database contained a SQL injection vulnerability.
I just got beaten up in HN for asking how the hell sql injection is still a problem. People get defensive, apparently.
People seem to take that much better.
You’re arguing semantics.
The two words are synonymous in most casual conversation where you would be in danger of offending by saying something is easy or simple.
Conversely, setting up Jira is neither straightforward, easy or simple.
Not even a few years ago I worked with people who insisted it was ok to write injection unsafe code if you knew for sure that you owned the injected values. Didn't matter that maybe one day that function would change to accept user-supplied data, that's not their problem! It was a Rails app and they were literally arguing wanting to do:
.where("id = #{id}")
.where("id = ?", id)
Imagine the internals of a database. An outer layer verifies some data is safe, and then all other functions assume it's safe.
The example you're sharing is a bit of straw man. It's just as easy to use the parameter, so of course that's the right thing. But interpolating a table name into the string from a constant isn't wrong.
I'm one of those people who moved from Ruby to Elixir. Ecto, Elixir's defacto database wrapper, will throw and exception if you try and write interpolated code like this, so luckily I don't have to have these insane arguments anymore (well, I work alone now, so there are several reasons I don't have to have them).
EDIT: My bad, I glazed past the last part of your statement.
Ya, I think this is probably where some of the defensiveness comes from: using a library vs rolling your own. If you're rolling your own, of course you're going to need to interpolate table names and whatnot, but it shouldn't even be possible to interpolate values. My example and argument is based of Rails, though, where you never specify a table name or anything like that. So in the specific case of my coworkers, they were wrong.
If you keep the preconditions informal and never check them, the code becomes brittle to modifications and refactoring. For a sufficiently large codebase you almost guarantee that at some point you will have a SQL injection bug.
That said, using prepared statements isn't the only way to guard against SQL injections. You can also use a query builder that will escape properly all data (provided this query builder itself is hardened against bugs). Using dynamic sql is the only way to make some kinds of queries, so a query builder is a must in those cases.
What you shouldn't do is to use string concatenation to build query strings in your business logic. It may or may not contain a bug right now, but it is brittle to changes in the codebase.
Most requirements can't be verified at compile time, or even at runtime in a feasible amount of time.
If you expect functions to do things that they don't say they do, I don't know what to tell you. Conventions and specs are the best we have.
> auditing it is awful.
If a function specifies a requirement, you look at the callers and see if that requirement is met. If it's easy to verify in code, you can assert. Is there an easier way to audit correctness?
Just like the meaning of life, it's best not to come to premature conclusions. Could all work out, or it could be a funny joke for aliens in the end.
If we're talking about a typed integer there is no chance of that turning into an sql injection attack.
If we're talking about a string, I'd probably insist on parameterizing it even if we completely own it just on the off chance that the future changes.
To draw an analogy, gun safety is important and everyone knows it. But I don't practice gun safety while watching television on my couch because the gun is locked away. I practice gun safety when I'm actually handling the thing that is dangerous.
And yes, I realize it being locked away is technically gun safety, it's an imperfect analogy, please roll with it.
I understand your point, I'm just saying if it's actually typed, it's safe.
It is a perfect analogy because you are practicing gun safety by locking the gun away. If someone that you are not expecting wanders into your home while you are sitting on the couch, such as a child, they will not suddenly have access to the firearm. This is exactly why you don't assume that you will never receive unsafe input in this situation.
IOW, you're free to make that claim and you're not wrong per se, but you're not right and it doesn't refute the point.
The number one rule of firearm safety - Treat every firearm as if it were loaded.
And yet children shoot themselves or others all the time because a gun was not safely stored.
But I digress...
Unless the database table switches to non-integer ids at some point.
In my defense, we trusted the input. But that's post-rationalisation, because I simply didn't know what I was doing at the time.
It gets worse. If I'd done it properly, my senior would have beaten me up in code review for "complexity". That was a man who would never use a screwdriver when a hammer was already in his hand.
His defense? "This system is internal only and never connected to the internet"
Senior titled devs don't necessarily know their shit.
If you promote the competent people, you leave the incompetent ones to do the actual work.
Breaking it down: That the most diligent / irreplaceable people who know the guts of the machine tend to be chained to their roles with occasional raises seems fairly logical from a C-Suite perspective. The tendency to promote incompetence - particularly overconfident incompetence - is the part that bears more scrutiny. If it were isolated to a few companies, it wouldn't be so relatable. I have a theory that it has to do with certain kinds of communication skills (specifically, bullshitting), being selected for in certain roles. And being able to write good code and explain why it has to be done that way requires the opposite of bullshitting.
The database has access control right? So only a few people in the org can read the data. And you are imagining a case where they:
a) find an inverse image of a password hash and use that login as another person to do something bad.
b) reverse the password from the hash to use in another context.
If a is an issue, why does this individual have sensitive data access in the first place? b is still unlikely. Any inverse image is unlikely to be the password if there is salting.
It sounds like an improvement could be made, but maybe not the highest priority. Can you inform me?
It's possible for developers to think they're actually doing the right thing, but it turns out they're not.
https://www.npmjs.com/package/mysql#escaping-query-values
> This looks similar to prepared statements in MySQL, however it really just uses the same connection.escape() method internally.
And depending on how the MySQL server is configured, connection.escape() can be bypassed.
So yeah, I'm coming from a PHP mindset where you can generally trust your engine to bind and escape values. My experience with Nodejs in this particular area caused me to write a lot of excess code (mostly to satisfy my own curiosity) and still convinced me not to trust it for the purpose.
In that light, I can understand how someone who jumped into the Nodejs ecosystem would think they were dealing with reliably safe escaping, and didn't realize what they were actually getting if they didn't read the fine print.