0

I'm super puzzled by this issue. I've looked up previous posts of 'incorrect syntax' but they had clear examples. I've got some C# code writing DbContext query code to my DB. I've got changing errors pointing at different characters in the same query code:

db.Database.ExecuteSqlCommand("INSERT INTO AspNetUsers (Id, Email, 
EmailConfirmed, PasswordHash, SecurityStamp, UserName, Location, First_Name, 
Last_Name, Bio, Online_Collaboration, Instrument, Genre, PhoneNumberConfirmed, 
TwoFactorEnabled, LockoutEnabled, AccessFailedCount) " +

"VALUES ('" + muser.Id + "', '" + muser.EmailAddress + "', 1, '" + 
muser.SecurityStamp + "', '" + muser.Username + "', '" + muser.Location + "', 
'" + muser.FirstName + "', '" + muser.LastName + "', '" + muser.Bio + "', 1, 
0, 0, 0, 0, 0, 0)");

The errors range. These are some samples below but that 'syntax near x' changes between mainly these letters: 

System.Data.SqlClient.SqlException: 'Incorrect syntax near 't'.'

System.Data.SqlClient.SqlException: 'Incorrect syntax near 'll'.
Unclosed quotation mark after the character string '', 1, 0, 0, 0, 0, 0, 0)'

System.Data.SqlClient.SqlException: 'Incorrect syntax near 'm'.
Unclosed quotation mark after the character string '', 1, 0, 0, 0, 0, 0, 0)'

System.Data.SqlClient.SqlException: 'Incorrect syntax near 's'.
Unclosed quotation mark after the character string '', 1, 0, 0, 0, 0, 0, 0)'

System.Data.SqlClient.SqlException: 'Incorrect syntax near 'm'.
Incorrect syntax near the keyword 'with'. If this statement is a common 
table expression, an xmlnamespaces clause or a change tracking context 
clause, the previous statement must be terminated with a semicolon.'

System.Data.SqlClient.SqlException: 'Incorrect syntax near 'll'.
Incorrect syntax near the keyword 'with'. If this statement is a common 
table expression, an xmlnamespaces clause or a change tracking context 
clause, the previous statement must be terminated with a semicolon.
Unclosed quotation mark after the character string '', 1, 0, 0, 0, 0, 0, 
0)'.'

What I've noticed so far:

  • Characters mainly s, t, m and also ll. The 'll' means it can't just be a single character issue. The characters are used frequently throughout the code so it's impossible to pinpoint although I'd imagine they're the symptom, not the cause.
  • The 'Incorrect syntax near x' is the only consistent error. I believe the others are just follow-ons. Of the dozens of times I've ran it, it's more the minimal message.
  • And finally the most frustrating part - it WORKS (very) infrequently so the syntax should be fine as far as I can see. Maybe it's shitting itself at the processioning end and some timings overlapping? There's a bit of string breaking for variables but I would imagine that's pretty standard load.
thatsHeavyDoc
  • 43
  • 4
  • 7
    You need to use a parameterized insert, currently if you have a `'` in a value things will break horribly and expose you to SQL Injection. – Alex K. Apr 16 '18 at 12:47
  • 2
    First thing to do: use **parameterizd queries** instead of inserting _user input_ directly into your query string. your code is open to [SQL Injection](http://www.bobby-tables.com)! – René Vogt Apr 16 '18 at 12:47
  • 1
    Your input includes a ', and you are wide open to SQL injection. If someone enters `', 1, 0, 0, 0, 0, 0, 0); DROP TABLE AspNetUsers; -- ` into their bio field, your user table will be gone. – ProgrammingLlama Apr 16 '18 at 12:50
  • 6
    [Obligatory xkcd reference](https://xkcd.com/327/) – Matthew Watson Apr 16 '18 at 12:52
  • I'm voting to close your question as a duplicate of this because parameterizing your query will fix your problem, and solve your huge SQL injection problem. – ProgrammingLlama Apr 16 '18 at 12:53
  • 1
    Parameterizing is definitely the way to go but hadn't you considered one of the very basics of debugging - rather than just staring at the code - print out the actual command string generated - look for the occurrences of things like 'll' or 'with' which do not appear in your code - it may become obvious exactly what the issue is. – PaulF Apr 16 '18 at 12:56

1 Answers1

7

Never create SQL queries/commands by concatenating strings. Not only does this make you vulnerable to SQL Injection, it can also cause problems with string escaping as you experienced first hand.

The correct way to build commands is to use SqlParameter.

var commandText = @"
INSERT INTO AspNetUsers 
    (Id, Email, EmailConfirmed, PasswordHash, SecurityStamp, UserName, 
     Location, First_Name, Last_Name, Bio, Online_Collaboration, Instrument,
     Genre, PhoneNumberConfirmed, TwoFactorEnabled, LockoutEnabled,
     AccessFailedCount)
VALUES
   (@id, @email, 1, @securityStamp, -- and so on for other values
   )
";

var idParameter = new SqlParameter("id", muser.Id); 
var emailParameter = new SqlParameter("email", muser.EmailAddress); 
var securityStampParameter = new SqlParameter("securityStamp", muser.SecurityStamp); 

var parameters = new [] { idParameter, emailParameter, securityStampParameter };

db.Database.ExecuteSqlCommand(commandText, parameters);
Georg Patscheider
  • 9,357
  • 1
  • 26
  • 36