0

We have an application where users can type search terms. For our needs, these can be separated by spaces, so the number of keywords is variable. Depending on each term, we will look for the exact keyword or use LIKE. We look for one OR the other and we query more than one table.

This question is mainly about how to mitigate SQL injection risk, but if a side-solution for the requirement removes this risk, that will work as well. I'm surely doing something wrong and/or missing something obvious, but cannot figure out what. I already considered:

  • using table parameters
  • parametrizing the tokens
  • trying to remove malicious characters/commands (in vain, of course)

Also already read as usual several blog posts and Q&As. Maybe the solution is one of the above, but then I would appreciate basic guidance about how to implement it concretely. Here is the minimum information to explain what we're trying to achieve:

User input (space separated, except between quotes):

A  "B*C D" EF* G

Tokens

A
B*C D
EF*
G

Clauses

= 'A'
LIKE '%B%C D%'
= 'D'
LIKE '%EF%'

Target query (ex. with 2 tables, A and B):

SELECT A.* FROM A WHERE A.a = 'A' OR LIKE '%B%C D%' ... (dynamic clauses)
UNION ALL
SELECT B.* FROM B WHERE B.a = 'A' OR LIKE '%B%C D%' ...

Notes

  • We're coding in C# and we can do whatever we want in code.
  • We're using Dapper.

Question
Is it possible to parametrize this? Else what would be the best strategy to build the query? Should we add functions that return the clauses, or split queries by table in different stored procedures...?

evilmandarine
  • 4,241
  • 4
  • 17
  • 40
  • i can't see any possible scenario for sql injection, as you split the whole thing in parts where you find a space and then parse it and if you find something that doesn't fit remove it. Eventually eliminate semicolons and other things that you don't want or need – nbk Feb 16 '23 at 14:58
  • 2
    @nbk you are suggesting a black list which is generally a [very bad idea](https://stackoverflow.com/questions/57690970/blacklist-whitelist-for-xss). Besides, there is not a single reason to do any filtering, the OP should use parameters, just like with any other data – Your Common Sense Feb 16 '23 at 15:05
  • I can see such a suggestion, like removing the semicolons, would result in a [Scunthorpe problem](https://en.wikipedia.org/wiki/Scunthorpe_problem). – Thom A Feb 16 '23 at 15:09
  • @YourCommonSense no whitelist, simply remove some unwanted single characters like semicolons, the rest is splittet and parsed, so no sql code can pass through without used as %Sqlinject% and would not yield any results – nbk Feb 16 '23 at 15:09
  • 1
    Should this question really be closed based on answer about EF, when the OP is using Dapper and has tagged it appropriately? – thewallrus Feb 16 '23 at 15:10
  • @nbk "remove some unwanted single characters" IS called a black list. And it should not be used in any security context. – Your Common Sense Feb 16 '23 at 15:10
  • @Larnu removing from single characters could result in problems, you described, but the semiclon was a suggestion, that must be examined by the user i don't know what exactly he wants to search – nbk Feb 16 '23 at 15:11
  • @thewallrus well I changed the dupe target, Hope now it fits. Someone more experienced in C# may try to google up a better dupe though, I won't object – Your Common Sense Feb 16 '23 at 15:14
  • How's this a duplicate of a question using EF? I am using Dapper, as it is mentioned in the question. – evilmandarine Feb 16 '23 at 15:16
  • @evilmandarine if you don't like the current dupe you can google up another one that would suit you better – Your Common Sense Feb 16 '23 at 15:20
  • There is no side-mitigation I think (challenge me to bypass them if you would pay if I do). Pass the whole thing a parameter to a stored procedure and do the rest there. We had almost the same necessity when full text search had problems, and we ended up creating CLR based stored procedure. It takes the whole thing as a parameter, parses etc. – Cetin Basoz Feb 16 '23 at 15:22
  • @CetinBasoz how a code in t-sql could be any better than a code in C#? – Your Common Sense Feb 16 '23 at 15:24
  • @YourCommonSense We're writing comments at the same time. I don't think all questions are the same, but your new link looks pretty much like what I was looking for. The previous EF link did not. – evilmandarine Feb 16 '23 at 15:24
  • @CetinBasoz This is what I tried to do in the beginning. But this becomes unmanageable very quickly in SQL, so we moved this logic to C# because it is much cleaner (and because we have this possibility of course). – evilmandarine Feb 16 '23 at 15:26
  • You could put your tokens in a derived table, then join that table on your target table. `declare @t varchar(16) = 'ASSOC'; declare @t2 varchar(16) = 'LLC'; select A.* from A inner join (values (@t), (@t2)) v(token) on A.name like '%' + v.token + '%';` – thewallrus Feb 16 '23 at 15:27
  • @YourCommonSense, maybe you didn't understand what I am saying. I am saying, don't even think there is a way to bypass SQL injectyion attack without using parameters. Pass parameters, then on SQL side what we did was CLR stored procedure or in other words C#. And sometimes T-SQL code could be better than C# code because it is already written in C and optimized for some specific thing. – Cetin Basoz Feb 16 '23 at 15:27
  • You can use C# in a SQL Server CLR stored procedure (if it were postgreSQL then you could use many languages). – Cetin Basoz Feb 16 '23 at 15:29
  • @CetinBasoz if you are building SQL dynamically in t-sql, then you are just passing the same SQL injection through parameters to t-sql. You aren't mitigating it, you are just moving it one level deeper. Parameters must be used for the every query. Now you have 2 queries, one is protected and one is not – Your Common Sense Feb 16 '23 at 15:29
  • @YourCommonSense, ok we have a communication problem, either I am not able to explain it or you don't understand. I don't know where I said build dynamic SQL. – Cetin Basoz Feb 16 '23 at 15:31
  • @CetinBasoz "*Pass the whole thing a parameter to a stored procedure and do the rest there*" <= here you said that. this query HAS to be dynamically built, in the procedure as well. Because it has dynamical number of parameters – Your Common Sense Feb 16 '23 at 15:33
  • @YourCommonSense, Ok, you know better. I didn't say that, you interpret like that. – Cetin Basoz Feb 16 '23 at 15:37
  • @CetinBasoz so you don't understand the question then. It involves a **variable number of WHERE conditions** based on user input. It can be either `WHERE A.a = LIKE '%B%C D%' ` OR `WHERE A.a = 'A' OR A.a = LIKE '%B%C D%'` OR whatever. It means SQL has to be built dynamically. With stored procedure or without. – Your Common Sense Feb 16 '23 at 15:48
  • @YourCommonSense, or maybe you don't understand it and have such a perception. That is not a valid SQL to start with, maybe that is why you think it needs to be dynamic. – Cetin Basoz Feb 16 '23 at 16:47

0 Answers0