1

I have a C# program that needs to process data and store it in SQL Server. The issue I am having is that some of the data being saved legitimately contains a single quote. When saving the data I need to locate the items that have single quotes in them and actively replace the single quote with two single quotes together so that I don't get a truncated string.

At present whereever I am adding data that might contain a single quote I am passing it through the following routine which I have in a static module called FSQ (Fix Single Quote), this is the routine:

/// <summary>
/// Fix Single Quote - Used to remove Double quotes from strings that would confuse Access database by replacing with Single Quotes.
/// </summary>
/// <param name="s">String text to be fixed by removing double quotes.</param>
/// <returns>The original string with any double-quotes removed and replaced with single quotes. If an error occurs will return an empty string.</returns>
public static string FSQ(string s)
{
    string tmp ="";

    try
    {
        if (s == null)
            return "";

        s = s.Trim();

        if (s == "")
            return s;

        if(s.Contains("'"))
        {
            if(!s.Contains("''"))//Already been fixed previously so skip here
                tmp = s.Replace("'", "''");

                s = tmp;
        }

        return s;


    }
    catch (Exception ex)
    {
        PEH("FDQ", "Common Module", ex.Message);
        return "";
    }
} //public static String FDQ(String s)

This works and my strings get saved OK to SQL but because there are a lot of calls to this routine, the performance sucks as the program loops through thousands of rows of data whilst it's processing.

Is there a more efficient routine that would negate the need to call this function? Mostly I am just building update or insert queries that contain these items.

Any help appreciated.

Panagiotis Kanavos
  • 120,703
  • 13
  • 188
  • 236
Siv
  • 125
  • 2
  • 14
  • 8
    Argh. `SqlCommand` + `SqlParamater` will make this a non-issue by handling escaping automatically, which is why you should always use them. – Alex K. May 18 '17 at 11:19
  • And also note how if I manage to pass in `'''` it skips performing the replacement and still allows an unmatched quote character through. – Damien_The_Unbeliever May 18 '17 at 11:22
  • 1
    @InnovaITveSolutions No, this shouldn't be used as a duplicate, because OP is doing it from C#. In this case escaping single quotes in a string is an invitation for SQL injection. – Sergey Kalinichenko May 18 '17 at 11:22
  • There is no way to fix FSQ, because this method itself is the bug. *DON'T* try to escape, just avoid string concatenation and you won't need quoting or any other such hacks. Parameterized queries can pass anything you want and are *simpler* to write – Panagiotis Kanavos May 18 '17 at 11:32
  • @Siv post the query and the code that executes it. It should be trivial to convert it to a parameterized query or use eg Dapper to convert it, eg to `conn.Query("SELECT * from Translations where word=@word and Date=@date",new {word="fo'c'sle",date=DateTime.Today};)` – Panagiotis Kanavos May 18 '17 at 11:33
  • 1
    @Panagiotis Kanavos: Couldn't you be persuaded to leave an answer? I'm a little worried that others looking for the answer won't see your comment, which is far better than the current two. – Palle Due May 18 '17 at 11:40
  • Thanks for all your comments. I do fully take the point about SQL Injection this is not public facing, but if using parameters also avoids the issue with single qotes in the string text then I am definitely up for it. – Siv May 18 '17 at 14:11

6 Answers6

2

NEVER EVER try to build SQL statements in your code. NEVER EVER try do do escape stuff yourself. You get at 100% a attackable code (sql injection).

Depending on your DB adapter you are using, you can use parameterized queries.

Here is a sample using ado.net:

var Query = "select * from customers where city = @city";
var cmd = new SqlCommand(Query);
cmd.Parameters.AddWithValue("@city", txtCity);

The @city in the query will be the placeholder, which is later replaced on driver level.

coding Bott
  • 4,287
  • 1
  • 27
  • 44
  • May I ask what will happend if someone will try to perform SQL injection attack? Will it "blow" inside try catch "cmd" has some property to be checked if everything is ok? – Arkadiusz Raszeja May 18 '17 at 12:32
  • It's not replaced at the driver level. When the command arrives at SQL Server, the command text and parameters are still distinct concepts. And that's what we want. SQL Server *knows* that the parameters are purely data and has no opportunity to misinterpret them as (parts of) the command. – Damien_The_Unbeliever May 18 '17 at 12:32
  • Yes you are right. I used that term for be more clear to beginners. – coding Bott May 18 '17 at 12:33
  • @Damien Thanks after reading everything here I am pretty much convinced that using parameters and getting rid of trying to strip out the single quotes is the best plan. – Siv May 18 '17 at 14:15
0

If you are passing it as a varchar/nvarchar parameter to sql server there will be no issue. You don't need to replace ' to ''.

Ravi
  • 1,157
  • 1
  • 9
  • 19
  • The data being processed is nvarchar and being stored as nvarchar as the data may include non - ascii characters. – Siv May 18 '17 at 14:33
  • then you have not to replace any character. Send string as it is. – Ravi May 19 '17 at 06:43
0

While dealing with string data you should be very cautions on how you handle it in SQL Server because of SQL Injection attack. SQL Inject Attack refers to an attack where the attacker inputs malformed text that will result into maliciously executing SQL Statements on your database.

Let's say you have a products table and following is the query you may have written to handle

var query = "SELECT * FROM Products WHERE ProductDesc = " + txtProductDesc;

As an attacker I may input "Smart Phones OR 1=1" and you can see that the query will end up listing all the products in the products table.

There are various sanitizing techniques you can follow to avert such attacks which you may want apply to handle such attacks

SQL Injection Prevention Cheat Sheet

Gururaj
  • 539
  • 2
  • 8
  • Nice to know. Thanks a lot for explanation – Arkadiusz Raszeja May 18 '17 at 12:11
  • May I ask about ORM's, like Entity framework or something like it? Are they safe or should I do something to prevent SQL injection attacks? – Arkadiusz Raszeja May 18 '17 at 12:22
  • Dangerous sample, because even you explain SQL injections the people tend to copy and paste code. Then they forget to filter. Using parameters is more save. – coding Bott May 18 '17 at 12:22
  • @BerndOtt - I understand the concern, but the person whoever is copying the code should read the text first and then proceed – Gururaj May 18 '17 at 12:25
  • Point is well taken - as mentioned elsewhere I am definitely going down the parameter line as a) it cures this issue (not that my code is anywhere public facing) and b) also saves me having to have the routine in my original question which is causing a big slowdown in time to process. – Siv May 18 '17 at 14:30
0

The first thing you need to do is to decouple processing code from persistance using Dependency Injection.
After that, your processed data will be exposed as a System.String and consumable by ANY DBMS via an appropriate interface.
Then, let's suppose you want to indeed want to persist to SQL Server. You are probably going to use Entity Framework, which will in turn handle escaping characters, as it specifically knows where it is going to store that data (of type string)...I suppose at least. This is what I would do! If someone more experienced than me believes this is the wrong way to go, please feel free to correct me!

Petros Apotsos
  • 615
  • 2
  • 6
  • 13
0

The following goes along with former replies in regards to using parameters rather than attempting to write your own method to handle apostrophes in your operation be it INSERT, DELETE or UPDATE. The following is simple, add a new record via parameters and return the new primary key.

The first insert has one apostrophe, the second three. The class below is in a class project while the second code block for the form is in a forms project which references the class project.

using System;
using System.Data.OleDb;
using System.IO;

namespace DataLib
{
    public class Operations1
    {
        public Exception InsertException { get; set; }
        private OleDbConnectionStringBuilder Builder = new OleDbConnectionStringBuilder
        {
            Provider = "Microsoft.ACE.OLEDB.12.0",
            DataSource = Path.Combine(AppDomain.CurrentDomain.BaseDirectory, "Database1.accdb")
        };
        public Operations1()
        {
            if (!(File.Exists(Builder.DataSource)))
            {
                throw new FileNotFoundException("Failed to find application's database");
            }
        }
        public bool AddNewRow(string CompanyName, string ContactName, ref int Identfier)
        {

            bool success = true;
            int affected = 0;

            try
            {
                using (OleDbConnection cn = new OleDbConnection { ConnectionString = Builder.ConnectionString })
                {
                    using (OleDbCommand cmd = new OleDbCommand { Connection = cn })
                    {
                        cmd.CommandText = @"INSERT INTO Customer (CompanyName,ContactName) 
                            VALUES (@CompanyName, @ContactName)";

                        cmd.Parameters.AddWithValue("@CompanyName", CompanyName);
                        cmd.Parameters.AddWithValue("@ContactName", ContactName);

                        cn.Open();

                        affected = cmd.ExecuteNonQuery();
                        if (affected == 1)
                        {
                            cmd.CommandText = "Select @@Identity";
                            Identfier = Convert.ToInt32(cmd.ExecuteScalar());
                            success = true;
                        }
                    }
                }
            }
            catch (Exception ex)
            {
                InsertException = ex;
                success = false;
            }
            return success;
        }
    }
}

Form code (mocked/static data)

using DataLib;
using System;
using System.Windows.Forms;

namespace DataLibDemo
{
    public partial class Form1 : Form
    {
        public Form1()
        {
            InitializeComponent();
        }
        private void button1_Click(object sender, EventArgs e)
        {
            Operations1 ops = new Operations1();
            int newIdentifier = 0;
            if (ops.AddNewRow("O'brien and company", "Mary O'brien", ref newIdentifier))
            {
                MessageBox.Show($"New Id for Mary {newIdentifier}");
            }
            else
            {
                MessageBox.Show($"Insert failed: {ops.InsertException.Message}");
            }

            if (ops.AddNewRow("O'''brien and company", "Mary O'brien", ref newIdentifier))
            {
                MessageBox.Show($"New Id for Mary {newIdentifier}");
            }
            else
            {
                MessageBox.Show($"Insert failed: {ops.InsertException.Message}");
            }
        }
    }
}

enter image description here

Karen Payne
  • 4,341
  • 2
  • 14
  • 31
  • Thanks for your comment, in this instance I am using SQL Server but your code highlights how to do things the parameter way which after reading all the commenst here is the way am going to fix my issue. – Siv May 18 '17 at 14:21
  • Hi, I would had given you SQL-Server but in your initial post there is a comment "confuse Access database" which I took for you using MS-Access. See my code sample below which is part of a larger MSDN code sample I did. https://code.msdn.microsoft.com/Adding-new-records-into-bff5eaaf/sourcecode?fileId=124530&pathId=772259264 – Karen Payne May 18 '17 at 14:24
-1

This the fastest way you can achieve.

public static string FSQ(string s)
{
       try
    {
        s = s.Trim();
        return s = (s != null && s != "")?(s.Contains("'") && !s.Contains("''"))?s.Replace("'", "''"): s:"";

    }
    catch (Exception ex)
    {
        PEH("FDQ", "Common Module", ex.Message);
        return "";
    }
} 
  • http://rextester.com/FFQP43481 – user8030929 May 18 '17 at 11:44
  • 1
    Still contains one of the many flaws of the original - it skips replacements if the string contains `''` so it allows `'''` to be passed through unmodified and thus you'd still have an un-escaped quote in this string. – Damien_The_Unbeliever May 18 '17 at 11:54
  • Thanks for this, although not the technique I will use as a result of posting here, it may be useful elsewhere in my kitbag of useful coding snippets, I think the overall consensus is to go for using SQL Parameters as it answers both the security aspect and the need to strip single quotes. Performance is my main concern as the way I am doing it currently is craeting a huge time overhead. – Siv May 18 '17 at 14:18