SQL Injection vulnerabilities with Excel import

Question

I’ve been researching a security report that was generated for an ASP.NET WebForms application. Some areas in our code have been noted to be potential SQL injection vulnerabilities. I have been looking into these areas in our codebase and they all appear to be where we are importing excel workbooks. The offending code seems to be when we are trying to create an OleDbCommand based on the name of the first sheet in the workbook. We are making a connection to the database (excel sheet) using an OleDbConnection object, which reads the database schema to find the name of the first sheet. We then dynamically construct a SQL command using that name. Example code might look like this:

    String connectionString = String.Format(
                "Provider=Microsoft.ACE.OLEDB.12.0;Data Source={0};" +
                "Extended Properties=\"Excel 12.0;HDR=NO;IMEX=1\"", sFullPath);
    OleDbConnection conn = new OleDbConnection(connectionString);
    conn.Open();
    DataTable dbSchema = conn.GetOleDbSchemaTable(OleDbSchemaGuid.Tables, null);
    string firstSheetName = dbSchema.Rows[0]["TABLE_NAME"].ToString();
    using (OleDbCommand command = new OleDbCommand("SELECT * FROM [" + firstSheetName+ "]", conn)) // Offending line of code

In an effort to remediate this SQL Injection vulnerability I have been looking into alternatives to the above code. Does anyone have any ideas how I might perform this without being flagged as a SQL Injection vulnerability? I do not want to use the ActiveX office objects to perform this operation.

Will simply moving this logic into a stored proc fix the vulnerability? — Edward Heyne, Oct 28 '16 at 15:19
the go-to defense, prepared statements, will NOT work here, because placeholders in prepared statements can only represent VALUES, not identifiers. You will have to use conventional sql escaping techniques. — Marc B, Oct 28 '16 at 15:19
Here's some information on SQL escaping techniques and related things: http://stackoverflow.com/questions/139199/can-i-protect-against-sql-injection-by-escaping-single-quote-and-surrounding-use — HaukurHaf, Oct 28 '16 at 15:22
@EdwardHeyne `Will simply moving this logic into a stored proc fix the vulnerability?` No. — Servy, Nov 15 '16 at 20:37

Dillon Ginley · Answer 1 · 2016-11-15T20:33:59.247

If you know the list of tables that are possible, you could create a whitelist of those tables, then check the spreadsheet names against that whitelist, as in the first example. You could also do a filtering String method validation as in the second example. You can also do a RegEx check, as in the last code example

Here is some sample code you might adapt to perform this filtering (I only left the whitelist check uncommented):

bool targetStringInArray = false;

 //Table name is in the whitelist
targetStringInArray = Array.Exists(tables, element => element == firstSheetName);

//Table Begins with begin with 'q'
//string[] tables = { "start", "q1","q3", "fyend", "q4"};
//targetStringInArray = Array.Exists(tables, firstSheetName=> firstSheetName.StartsWith("q"));

//Table name is only letters, numbers and underscores:
//targetStringInArray = Regex.IsMatch(firstSheetName, @"^[a-zA-Z0-9_]+$");



if (targetStringInArray)
{
  using (OleDbCommand command = new OleDbCommand("SELECT * FROM [" + firstSheetName+ "]", conn)) // Offending line of code
}

SQL Injection vulnerabilities with Excel import

1 Answers1