6

I'm just not finding any answers I like out there.

I would like to do something like:

public class TestSqlInjectionController : ApiController
{
    public IEnumerable<TestSqlInjectionUser> Get([ValidateSqlInjection]string usernameFilter = null)
    {

where [ValidateSqlInjection] looks for basics like throwing an error if the incoming filter contains ;, --, DROP, or DELETE.

So I'd have a maintainable list.

Then create a custom attribute:

[FilterField1ValidateSqlInjection]

Here maybe split a comma-delimited list into an array.

Then roll through the array and make sure each element matches one of the values in an enum.

Does this sound like it's heading in the right direction?

The problem is that we have to use dynamic SQL for a lot of paging, sorting and filtering. There is no way around it.

So command parameters don't really do a whole lot for us.

Does the custom property attributes sound like a good idea to anyone out there with .Net SQL Injection experience?

marc_s
  • 732,580
  • 175
  • 1,330
  • 1,459
Sam
  • 4,766
  • 11
  • 50
  • 76
  • 5
    This is a classic black-list approach - which is **flawed** since injectors are so darn ingenious... every time you add another keyword or set of characters to check for, those pesky attackers already have 10 more up their sleeve - it's a hopeless up-hill battle. So really, you should **either** use a proper technique like regular SQL with parameterized queries (which I think is by far the preferred way to go - and it can easily handle paging, sorting, filtering - trust me!), or then you should much rather use a **white-list** approach with a list of valid keywords – marc_s Nov 23 '15 at 22:05
  • You should look for [user input escaping](https://www.owasp.org/index.php/SQL_Injection_Prevention_Cheat_Sheet#Defense_Option_3:_Escaping_All_User_Supplied_Input) – Bruno Costa Nov 23 '15 at 22:08
  • What is `usernameFilter` supposed to contain? Why not accept a collection of strings instead of a single string? – D Stanley Nov 23 '15 at 22:33

4 Answers4

2

The best option is to use SQL parameters, but since it is not sufficient to you I would try to use a SQL executor like pattern.

Instead you try to decorate all your possible inputs with an attribute, create one helper class that is responsible to execute all your dynamic queries and inside the execution method test if it has any SQL injection pattern. You can do this with regex like this example and throw an exception if found.

On your UI you can catch this exception and show some output to your user.

Luiz
  • 542
  • 5
  • 10
0

There are many solutions available. Here are some of the top search results from google.

https://msdn.microsoft.com/en-us/library/ff648339.aspx Control SQL injection in MVC http://devproconnections.com/net-framework/protecting-legacy-web-applications-antixss

The best option however to deal with SQL injection is to not use dynamic SQL. There are much better tools available to you today. There are parameterized SQL statements and store procedures. However, if your query absolutely must be dynamic, I recommend that you consider using linq and expression trees.

https://msdn.microsoft.com/en-us/library/bb882637.aspx

Community
  • 1
  • 1
tdbeckett
  • 618
  • 8
  • 19
  • Although I certainly agree with you, that Dynamic SQL should be avoided where possible, there's at least a handful of cases where it plainly can't be avoided. – user2366842 Nov 23 '15 at 22:21
  • Again I point to Expression trees. The can build dynamic SQL statements that aren't easily susceptible to Injection attacks. Just check out the last link in the answer. – tdbeckett Nov 23 '15 at 22:27
0

To be completely safe and dynamic you would need to write a parser that parses the filter string correcly, but I would instead advice you to use a dynamic Linq library, with the library methods your client code could actually can run EF queries from dynamically built lambda expressions which themselves the library methods constructs by parsing user input string. This method produces safe query parts which can only be used in the ORDER BY, WHERE or SELECT clause.

Here's a link to such a library: https://github.com/NArnott/System.Linq.Dynamic

Some docs for this library: http://dynamiclinq.azurewebsites.net/GettingStarted#subValues

Kaveh Hadjari
  • 217
  • 1
  • 10
0

Aspect oriented is your solution. changing the project will be too painful. find a good parser that checks the input string for weird behaviour and remove suspicious strings if observed.

[CleanInputArgs]
public void CallServer(string sqlarg) 
{
    // logic...
}

public void CleanInputArgs(args)
{
    // here is the thing. iterate over args and 
    // remove all suspicious strings that might be SQL injection
}
Ami Vaknin
  • 194
  • 6