Is this sanitization unsafe? Is it vulnerable to SQL Injection?

Question

Function RemoveSuspeitos(ByVal strTXT)
               Dim txtAux As String
               txtAux = strTXT
               txtAux = Replace(txtAux, chr(34), "")
               txtAux = Replace(txtAux, "'", "")
               RemoveSuspeitos = txtAux
End Function

DB: MSSQL

1) Forget syntax errors in the above code, I am not expert in VB.

2) Lets say I always use single or double quotes, even for int values (e.g.: '" + $int_id + "').

Is this sanitization unsafe? If yes, why? Please show me a real exploit scenario.

Answer 1

Here is my try.

The problem with vulnerabilities is that they are not that direct as most users think.

In reality, production code grows slightly different from the artificial, totally controlled example.

So, here not technological, but methodological vulnerabilities are coming into the scene. I made a vast effort researching this matter last year, and here are my conclusions

Let's start from a metaphor:

One have to always use a seat belt. No exceptions. Yes, you can always say that you drive so safely that no crash ever be possible. But, unfortunately, statistics is against you. There are other people involved in the traffic. There are sudden obstacles. There are unforeseen breakages. That's why you should always wear a seat belt. Exactly the same thing with protecting your code.

Here let me post an excerpt from my research:

---

Why manual formatting is bad?

Because it's manual. Manual == error prone. It depends on the programmer's skill, temper, mood, number of beers last night and so on. As a matter of fact, manual formatting is the very and the only reason for the most injection cases in the world. Why?

1. Manual formatting can be incomplete.
   Let's take Bobby Tables' case. It's a perfect example of incomplete formatting: string we added to the query were quoted but not escaped! While we just learned from the above that quoting and escaping should be always applied together (along with setting the proper encoding for the escaping function). But in a usual PHP application which does SQL string formatting separately (partly in the query and partly somewhere else), it is very likely that some part of formatting may be simple overlooked.

2. Manual formatting can be applied to wrong literal.
   Not a big deal as long as we are using complete formatting (as it will cause immediate error which can be fixed at development phase), but combined with incomplete formatting it's a real disaster. There are hundreds of answers on the great site of Stack Overflow, suggesting to escape identifiers the same way as strings. Which is totally useless and leads straight to injection.

3. Manual formatting is essentially non-obligatory measure.
   First of all, there is obvious lack of attention case, where proper formatting can be simply forgotten. But there is a real weird case - many PHP users often intentionally refuse to apply any formatting, because up to this day they still separating data to "clean" and "unclean", "user input" and "non-user input", etc. Means "safe" data don't require formatting. Which is a plain nonsense - remember Sarah O'Hara. From the formatting point of view, it is destination that matters. A developer have to mind the type of SQL literal, not the data source. Is this string going to the query? It have to be formatted then. No matter, if it is from user input or just mysteriously appeared amidst the code execution.

4. Manual formatting can be separated from the actual query execution by considerable distance.

   Most underestimated and overlooked issue. Yet most essential of them all, as it alone can spoil all the other rules, if not followed.
   Almost every PHP user is tempted to do all the "sanitization" in one place, far away from the actual query execution, and this false approach is a source of innumerable faults:

   • first of all, having no query at hand, one cannot tell what kind of SQL literal this certain piece of data is going represent - and thus violate both formatting rules (1) and (2) at once.
   • having more than one place for santization, we're calling for disaster, as one developer would think it was done by another, or made already somewhere else, etc.
   • having more than one place for santization, we're introducing another danger, of double-sanitizing data (say, one developer formatted it at the entry point and another - before query execution)
   • premature formatting most likely will spoil the source variable, making it unusable anywhere else.

---

As you can see, first two items can be considered inapplicable if, as you say, you're always putting your values in quotes. But here come last two. "Always" is too presuming a word. We are humans and we all make mistakes. You are not only one who is working on the project. And even if you personally doing everything right, other users may not share your confidence. Say, some of them may share a widespread delusion as though only user input have to be "sanitized" - and thus expose to the danger of second order injection.

This is why there ought to be mechanism that may guarantee 100% safety if strictly followed, no matter if developer understands it or not. And using placeholders for the EVERY dynamical literal in the query IS such a mechanism.