Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

String values are not escaped in CONTAINS function #667

Closed
wiredbarb opened this issue Aug 14, 2018 · 4 comments · Fixed by #670
Closed

String values are not escaped in CONTAINS function #667

wiredbarb opened this issue Aug 14, 2018 · 4 comments · Fixed by #670

Comments

@wiredbarb
Copy link
Collaborator

Steps to reproduce

Executing

var groupNames = new[] {"mydomain\\group1", "mydomain\\group2"};
var existingGroups = await dbContext.Groups.AsNoTracking()
    .Where(x => groupNames.Contains(x.Name)).Select(x => x.Name)
    .ToArrayAsync();

(with Name being a string field) resolves to

SELECT `x`.`Name`
FROM `Groups` AS `x`
WHERE `x`.`Name` IN ('mydomain\group1', 'mydomain\group2')

when the expected resulting SQL query would be

[...] WHERE `x`.`Name` IN ('mydomain\\group1', 'mydomain\\group2')

The issue

In my case, this only results in zero rows regardless whether those values exist in the database.
Potentially this opens vulnerabilites for SQL injection attacks whenever developers rely on input being escaped by the EF connector (i. e. a lot, I suppose). I have not checked whether other queries are affected as well or if this behaviour is limited to contains function calls.

Further technical details

Pomelo.EntityFrameworkCore.MySql version: 2.1.0

@mguinness
Copy link
Collaborator

MySQL (unlike other databases) allows limited special character escape sequences in string literals. You can avoid this by using NO_BACKSLASH_ESCAPES.

@wiredbarb
Copy link
Collaborator Author

Thanks, but setting server side modes globally might not be an option for everyone - consider shared database scenarios, for instance. How can this be provided session-wise?

Plus, I am still convinced that trusting in EF connectors to generally abstract the database behaviour (aside from advertised limitations) is quite common. So, unless stated otherwise in the connector's documentation, linq queries are expected to be safe without further action:

Yes, LINQ to SQL helps to stop SQL injection, because it passes all data to the database via SQL parameters. LINQ queries are not composed by using string manipulation or concatenation, that's why they are not susceptible to traditional SQL injection attacks.
http://entityframework.net/linq-prevent-sql-injection

So in fact I have to correct the expected result mentioned in the issue - it should reference parameters instead of providing escaped input.

@mguinness
Copy link
Collaborator

mguinness commented Aug 15, 2018

SQL mode can be set at the session level using SET @@SQL_MODE. See Can I fire an event on connect database in Entity Framework Core? for a way to do it inside EF.

We also welcome PR's, see Call for Contributors for more details. It would also be of interest to see if the MySql.Data.EntityFrameworkCore escapes that input, could you test against that provider?

@wiredbarb
Copy link
Collaborator Author

MySql.Data.EntityFrameworkCore behaves exactly the same: No parameters, no escaping.

I'll try and come up with a solution proposal in the next few days.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants