0

I have found out that our software is vulnerable to sql-injection.

Our Software is written with C# and a kind of the Gizmox framework. Our framework can’t deal with parameterized queries and uses string concatenation to build the sql queries.

I know that this all sounds very bad, but on the long term we will refactor this. On the short term we need a quick (and maybe dirty) hotfix.

Our idea is to validate the userinput against the following blacklist with regex:

"--" , ";--" , ";" , "/*" , "*/" , "@@" , "@" , "char" , "nchar" , "varchar" , "nvarchar" , "alter" , "begin" , "cast" , "create" , "cursor" , "declare" , "delete" , "drop" , "end" , "exec" , "execute" , "fetch" , "insert" , "kill" , "open" , "select" , "sys" , "sysobjects" , "syscolumns" , "table" , "update"

Due to the fact that a blacklist is the weakest sql protection that you can have, I want to ask for a better solution.

TheSchmu
  • 574
  • 5
  • 25
Bambuk
  • 192
  • 1
  • 15
  • https://www.owasp.org/index.php/SQL_Injection_Prevention_Cheat_Sheet – walther Jun 05 '15 at 09:59
  • 6
    The better solution is to stop, **right now** and refactor out the string concatenation. ***Now***. At least, go do that in the easier cases (the ones which most clearly map to using SQL parameters). Later you can go back to the ones where you would need to parameterize `IN` or `WHERE` clauses and such. – John Saunders Jun 05 '15 at 10:01
  • Prepared Statements (Parameterized Queries) will be solution for the long term, because it will be a huge amount of work. Stored Procedures are not allowed (not my decision) – Bambuk Jun 05 '15 at 10:03
  • 1
    Does your company have any of _my_ data stored? If not, then I guess I won't care when the hackers get you. Otherwise, your organization is being foolish by waiting for the long term before becoming minimally secure. You may not still be in business in the "long term". – John Saunders Jun 05 '15 at 10:07
  • 5
    *"Our framework can't handle parameterized queries"* I'd heartily suggest you got a new framework that can. – John Bell Jun 05 '15 at 10:14
  • Oh dear. You are already vulnerable. This solution will not close all the gaps, could give false positives, and how are you going to message the user about why they can't use the word begin. Waste of effort get on with the proper solution. – Tony Hopkinson Jun 05 '15 at 10:24
  • Find a hacker for hire or a new job. the first option might be better. Using concatenated strings is extreamly vulnerable, a blacklist is a **bad** solution, and using sql server but not alowing stored procedures is like driving a ferrary in a school zone. You better find a way to educate your boss or a better boss. – Zohar Peled Jun 05 '15 at 11:27
  • "*but on the long term we will refactor this*" This is one of the worst lies one can say to himself :D 99% of the times, that "long term" never comes. – Alejandro Jun 05 '15 at 19:18
  • 1
    On a more constructive comment, I would say your blacklist approach is wrong, as the SQL reserved keywords can also be legitimate words for the system in case. I would look into *at the very least* escaping all single quotes with two single quotes. That *quick and dirty* patch (**and certainly insufficient**) would at least allow other input unrestricted, so that input with quotes don't cause SQL errors,as the quote itself would enter the DB as part of the data. – Alejandro Jun 05 '15 at 19:22

0 Answers0