SQL Injection: Calling Stored Procedures Dynamically

October 26, 2016 by · Comments Off on SQL Injection: Calling Stored Procedures Dynamically
Filed under: Development, Security, Testing 

It is not news that SQL Injection is possible within a stored procedure. There have been plenty of articles discussing this issues. However, there is a unique way that some developers execute their stored procedures that make them vulnerable to SQL Injection, even when the stored procedure itself is actually safe.

Look at the example below. The code is using a stored procedure, but it is calling the stored procedure using a dynamic statement.

	conn.Open();
        var cmdText = "exec spGetData '" + txtSearch.Text + "'";
        SqlDataAdapter adapter = new SqlDataAdapter(cmdText, conn);
        DataSet ds = new DataSet();
        adapter.Fill(ds);
        conn.Close();
        grdResults.DataSource = ds.Tables[0];
        grdResults.DataBind();

It doesn’t really matter what is in the stored procedure for this particular example. This is because the stored procedure is not where the injection is going to occur. Instead, the injection occurs when the EXEC statement is concatenated together. The email parameter is being dynamically added in, which we know is bad.

This can be quickly tested by just inserting a single quote (‘) into the search field and viewing the error message returned. It would look something like this:

System.Data.SqlClient.SqlException (0x80131904): Unclosed quotation mark after the character string ”’. at System.Data.SqlClient.SqlConnection.OnError(SqlException exception, Boolean breakConnection, Action`1 wrapCloseInAction) at System.Data.SqlClient.SqlInternalConnection.OnError(SqlException exception, Boolean breakConnection, Action`1 wrapCloseInAction) at System.Data.SqlClient.TdsParser.ThrowExceptionAndWarning(TdsParserStateObject stateObj, Boolean callerHasConnectionLock, Boolean asyncClose) at System.Data.SqlClient.TdsParser.TryRun(RunBehavior runBehavior, SqlCommand cmdHandler, SqlDataReader dataStream, BulkCopySimpleResultSet bulkCopyHandler, TdsParserStateObject stateObj, Boolean& dataReady) at System.Data.SqlClient.SqlDataReader.TryConsumeMetaData() at System.Data.SqlClient.SqlDataReader.get_MetaData() at

With a little more probing, it is possible to get more information leading us to understand how this SQL is constructed. For example, by placing ‘,’ into the search field, we see a different error message:

System.Data.SqlClient.SqlException (0x80131904): Procedure or function spGetData has too many arguments specified. at System.Data.SqlClient.SqlConnection.

The mention of the stored procedure having too many arguments helps identify this technique for calling stored procedures.

With SQL we have the ability to execute more than one query in a given transaction. In this case, we just need to break out of the current exec statement and add our own statement. Remember, this doesn’t effect the execution of the spGetData stored procedure. We are looking at the ability to add new statements to the request.

Lets assume we search for this:

james@test.com’;SELECT * FROM tblUsers–

this would change our cmdText to look like:

exec spGetData’james@test.com’;SELECT * FROM tblUsers–‘

The above query will execute the spGetData stored procedure and then execute the following SELECT statement, ultimately returning 2 result sets. In many cases, this is not that useful for an attacker because the second table would not be returned to the user. However, this doesn’t mean that this makes an attack impossible. Instead, this turns our attacks more towards what we can Do, not what can we receive.

At this point, we are able to execute any commands against the SQL Server that the user has permission too. This could mean executing other stored procedures, dropping or modifying tables, adding records to a table, or even more advanced attacks such as manipulating the underlying operating system. An example might be to do something like this:

james@test.com’;DROP TABLE tblUsers–

If the user has permissions, the server would drop tblUsers, causing a lot of problems.

When calling stored procedures, it should be done using command parameters, rather than dynamically. The following is an example of using proper parameters:

    conn.Open();
    SqlCommand cmd = new SqlCommand();
    cmd.CommandText = "spGetData";
    cmd.CommandType = CommandType.StoredProcedure;
    cmd.Connection = conn;
    cmd.Parameters.AddWithValue("@someData", txtSearch.Text);
    SqlDataAdapter adapter = new SqlDataAdapter(cmd);
    DataSet ds = new DataSet();
    adapter.Fill(ds);
    conn.Close();
    grdResults.DataSource = ds.Tables[0];
    grdResults.DataBind();

The code above adds parameters to the command object, removing the ability to inject into the dynamic code.

It is easy to think that because it is a stored procedure, and the stored procedure may be safe, that we are secure. Unfortunately, simple mistakes like this can lead to a vulnerability. Make sure that you are properly making database calls using parameterized queries. Don’t use dynamic SQL, even if it is to call a stored procedure.

XXE and .Net

May 26, 2016 by · Comments Off on XXE and .Net
Filed under: Development, Security 

XXE, or XML External Entity, is an attack against applications that parse XML. It occurs when XML input contains a reference to an external entity that it wasn’t expected to have access to. Through this article, I will discuss how .Net handles XML for certain objects and how to properly configure these objects to block XXE attacks. It is important to understand that the different versions of the .Net framework handle this differently. I will point out the differences for each object.

I will cover the XmlReader, XmlTextReader, and XMLDocument. Here is a quick summary regarding the default settings:

Object Safe by Default?
XmlReader  
Prior to 4.0 Yes
4.0 + Yes
XmlTextReader  
Prior to 4.0 No
4.0 + No
XmlDocument  
4.5 and Earlier No
4.6 Yes

XMLReader

Prior to 4.0

The ProhibitDtd property is used to determine if a DTD will be parsed.

  • True (default) – throws an exception if a DTD is identified. (See Figure 1)
  • False – Allows parsing the DTD. (Potentially Vulnerable)

Code that throws an exception when a DTD is processed: – By default, ProhibitDtd is set to true and will throw an exception when an Entity is referenced.

static void Reader()
{
    string xml = "<?xml version=\"1.0\" ?><!DOCTYPE doc 
	[<!ENTITY win SYSTEM \"file:///C:/Users/user/Documents/testdata2.txt\">]
	><doc>&win;</doc>";

    XmlReader myReader = XmlReader.Create(new StringReader(xml));
            
    while (myReader.Read())
    {
        Console.WriteLine(myReader.Value);
    }
    Console.ReadLine();
}

Exception when executed:

[Figure 1]

XXE 1

Code that allows a DTD to be processed: – Using the XmlReaderSettings object, it is possible to allow the parsing of the entity. This could make your application vulnerable to XXE.

static void Reader()
{
    string xml = "<?xml version=\"1.0\" ?><!DOCTYPE doc 
	[<!ENTITY win SYSTEM \"file:///C:/Users/user/Documents/testdata2.txt\">]
	><doc>&win;</doc>";

    XmlReaderSettings rs = new XmlReaderSettings();

    rs.ProhibitDtd = false;

    XmlReader myReader = XmlReader.Create(new StringReader(xml),rs);
            
    while (myReader.Read())
    {
        Console.WriteLine(myReader.Value);
    }
    Console.ReadLine();
}

Output when executed showing injected text:

[Figure 2]

XXE 2

.Net 4.0+
In .Net 4.0, they made a change from using the ProhibitDtD property to the new DtdProcessing enumeration. There are now three (3) options:

  • Prohibit (default) – Throws an exception if a DTD is identified.
  • Ignore – Ignores any DTD specifications in the document, skipping over them and continues processing the document.
  • Parse – Will parse any DTD specifications in the document. (Potentially Vulnerable)

Code that throws an exception when a DTD is processed: – By default, the DtdProcessing is set to Prohibit, blocking any external entities and creating safe code.

static void Reader()
{
    string xml = "<?xml version=\"1.0\" ?><!DOCTYPE doc 
	[<!ENTITY win SYSTEM \"file:///C:/Users/user/Documents/testdata2.txt\">]
	><doc>&win;</doc>";

    XmlReader myReader = XmlReader.Create(new StringReader(xml));
            
    while (myReader.Read())
    {
        Console.WriteLine(myReader.Value);
    }
    Console.ReadLine();
}

Exception when executed:

[Figure 3]

XXE 3

Code that ignores DTDs and continues processing: – Using the XmlReaderSettings object, setting DtdProcessing to Ignore will skip processing any entities. In this case, it threw an exception because there was a reference to the entirety that was skipped.

static void Reader()
{
    string xml = "<?xml version=\"1.0\" ?><!DOCTYPE doc 
	[<!ENTITY win SYSTEM \"file:///C:/Users/user/Documents/testdata2.txt\">]
	><doc>&win;</doc>";

    XmlReaderSettings rs = new XmlReaderSettings();
    rs.DtdProcessing = DtdProcessing.Ignore;

    XmlReader myReader = XmlReader.Create(new StringReader(xml),rs);
            
    while (myReader.Read())
    {
        Console.WriteLine(myReader.Value);
    }
    Console.ReadLine();
}

Output when executed ignoring the DTD (Exception due to trying to use the unprocessed entity):

[Figure 4]

XXE 4

Code that allows a DTD to be processed: Using the XmlReaderSettings object, setting DtdProcessing to Parse will allow processing the entities. This potentially makes your code vulnerable.

static void Reader()
{
    string xml = "<?xml version=\"1.0\" ?><!DOCTYPE doc 
	[<!ENTITY win SYSTEM \"file:///C:/Users/user/Documents/testdata2.txt\">]
	><doc>&win;</doc>";
			
    XmlReaderSettings rs = new XmlReaderSettings();
    rs.DtdProcessing = DtdProcessing.Parse;

    XmlReader myReader = XmlReader.Create(new StringReader(xml),rs);
            
    while (myReader.Read())
    {
        Console.WriteLine(myReader.Value);
    }
    Console.ReadLine();           
}

Output when executed showing injected text:

[Figure 5]

XXE 5


XmlTextReader

The XmlTextReader uses the same properties as the XmlReader object, however there is one big difference. The XmlTextReader defaults to parsing XML Entities so you need to explicitly tell it not too.

Prior to 4.0

The ProhibitDtd property is used to determine if a DTD will be parsed.

  • True – throws an exception if a DTD is identified. (See Figure 1)
  • False (Default) – Allows parsing the DTD. (Potentially Vulnerable)

Code that allows a Dtd to be processed: (Potentially Vulnerable) – By default, the XMLTextReader sets the ProhibitDtd property to False, allowing entities to be parsed and the code to potentially be vulnerable.

static void TextReader()
{
    string xml = "<?xml version=\"1.0\" ?><!DOCTYPE doc 
	[<!ENTITY win SYSTEM \"file:///C:/Users/user/Documents/testdata2.txt\">]
	><doc>&win;</doc>";

    XmlTextReader myReader = new XmlTextReader(new StringReader(xml));

    while (myReader.Read())
    {
         if (myReader.NodeType == XmlNodeType.Element)
         {
             Console.WriteLine(myReader.ReadElementContentAsString());
         }
    }
    Console.ReadLine();
}

Code that blocks the Dtd from being parsed and throws an exception: – Setting the ProhibitDtd property to true (explicitly) will block Dtds from being processed making the code safe from XXE. Notice how the XmlTextReader has the ProhibitDtd property directly, it doesn’t have to use the XmlReaderSettings object.

static void TextReader()
{
    string xml = "<?xml version=\"1.0\" ?><!DOCTYPE doc 
	[<!ENTITY win SYSTEM \"file:///C:/Users/user/Documents/testdata2.txt\">]
	><doc>&win;</doc>";

    XmlTextReader myReader = new XmlTextReader(new StringReader(xml));

    myReader.ProhibitDtd = true;

    while (myReader.Read())
    {
       if (myReader.NodeType == XmlNodeType.Element)
       {
           Console.WriteLine(myReader.ReadElementContentAsString());
       }
    }
    Console.ReadLine();
}

4.0+

In .Net 4.0, they made a change from using the ProhibitDtD property to the new DtdProcessing enumeration. There are now three (3) options:

  • Prohibit – Throws an exception if a DTD is identified.
  • Ignore – Ignores any DTD specifications in the document, skipping over them and continues processing the document.
  • Parse (Default) – Will parse any DTD specifications in the document. (Potentially Vulnerable)

Code that allows a DTD to be processed: (Vulnerable) – By default, the XMLTextReader sets the DtdProcessing to Parse, making the code potentially vulnerable to XXE.

static void TextReader()
{
    string xml = "<?xml version=\"1.0\" ?><!DOCTYPE doc 
	[<!ENTITY win SYSTEM \"file:///C:/Users/user/Documents/testdata2.txt\">]
	><doc>&win;</doc>";

    XmlTextReader myReader = new XmlTextReader(new StringReader(xml));

    while (myReader.Read())
    {
        if (myReader.NodeType == XmlNodeType.Element)
        {
            Console.WriteLine(myReader.ReadElementContentAsString());
        }
    }
    Console.ReadLine();
}

Code that blocks the Dtd from being parsed: – To block entities from being parsed, you must explicitly set the DtdProcessing property to Prohibit or Ignore. Note that this is set directly on the XmlTextReader and not through the XmlReaderSettings object.

static void TextReader()
{
    string xml = "<?xml version=\"1.0\" ?><!DOCTYPE doc 
	[<!ENTITY win SYSTEM \"file:///C:/Users/user/Documents/testdata2.txt\">]
	><doc>&win;</doc>";

    XmlTextReader myReader = new XmlTextReader(new StringReader(xml));
			
    myReader.DtdProcessing = DtdProcessing.Prohibit;

    while (myReader.Read())
    {
         if (myReader.NodeType == XmlNodeType.Element)
         {
             Console.WriteLine(myReader.ReadElementContentAsString());
         }
    }
    Console.ReadLine();
}

Output when Dtd is prohibited:

[Figure 6]

XXE 6


XMLDocument

For the XMLDocument, you need to change the default XMLResolver object to prohibit a Dtd from being parsed.

.Net 4.5 and Earlier

By default, the XMLDocument sets the URLResolver which will parse Dtds included in the XML document. To prohibit this, set the XmlResolver = null.

Code that does not set the XmlResolver properly (potentially vulnerable) – The default XMLResolver will parse entities, making the following code potentially vulnerable.

static void Load()
{
   string fileName = @"C:\Users\user\Documents\test.xml";

   XmlDocument xmlDoc = new XmlDocument();

   xmlDoc.Load(fileName);

   Console.WriteLine(xmlDoc.InnerText);

   Console.ReadLine();
}

Code that does set the XmlResolver to null, blocking any Dtds from executing: – To block entities from being parsed, you must explicitly set the XmlResolver to null. This example uses LoadXml instead of Load, but they both work the same in this case.

static void LoadXML()
{
   string xml = "<?xml version=\"1.0\" ?><!DOCTYPE doc 
	[<!ENTITY win SYSTEM \"file:///C:/Users/user/Documents/testdata2.txt\">]
	><doc>&win;</doc>";

   XmlDocument xmlDoc = new XmlDocument();

   xmlDoc.XmlResolver = null;

   xmlDoc.LoadXml(xml);

   Console.WriteLine(xmlDoc.InnerText);

   Console.ReadLine();
}

.Net 4.6

It appears that in .Net 4.6, the XMLResolver is defaulted to Null, making the XmlDocument safe. However, you can still set the XmlResolver in a similar way as prior to 4.6 (see previous code snippet).

A Pen Test is Coming!!

October 18, 2014 by · Comments Off on A Pen Test is Coming!!
Filed under: Development, Security, Testing 

You have been working hard to create the greatest app in the world.  Ok, so maybe it is just a simple business application, but it is still important to you.  You have put countless hours of hard work into creating this master piece.  It looks awesome, and does everything that the business has asked for.  Then you get the email from security: Your application will undergo a penetration test in two weeks.  Your heart skips a beat and sinks a little as you recall everything you have heard about this experience.  Most likely, your immediate action is to go on the defensive.  Why would your application need a penetration test?  Of course it is secure, we do use HTTPS.  No one would attack us, we are small.  Take a breath..  it is going to be alright.

All too often, when I go into a penetration test, the developers start on the defensive.  They don’t really understand why these “other” people have to come in and test their application.  I understand the concerns.   History has shown that many of these engagements are truly considered adversarial.  The testers jump for joy when they find a security flaw.  They tell you how bad the application is and how simple the fix is, leading to you feeling about the size of an ant.  This is often due to a lack of good communication skills. 

Penetration testing is adversarial.  It is an offensive assessment to find security weaknesses in your systems.  This is an attempt to simulate an attacker against your system.  Of course there are many differences, such as scope, timing and rules, but the goal is the same.  Lets see what we can do on your system.  Unfortunately, I find that many testers don’t have the communication skills to relay the information back to the business and developers in a way that is positive.  I can’t tell you how may times I have heard people describe their job as great because they get to come in, tell you how bad you suck and then leave.  If that is your penetration tester, find a new one.  First, that attitude breaks down the communication with the client and doesn’t help promote a secure atmosphere.  We don’t get anywhere by belittling the teams that have worked hard to create their application.  Second, a penetration test should provide solid recommendations to the client on how they can work to resolve the issues identified.  Just listing a bunch of flaws is fairly useless to a company. 

These engagements should be worth everyone’s time.  There should be positive communication between the developers and the testing team.  Remember that many engagements are short lived so the more information you can provide the better the assessment you are going to get.  The engagement should be helpful.  With the right company, you will get a solid assessment and recommendations that you can do something with.  If you don’t get that, time to start looking at another company for testing.  Make sure you are ready for the test.   If the engagement requires an environment to test in, have it all set up.  That includes test data (if needed).   The testers want to hit the ground running.  If credentials are needed, make sure those are available too.  The more help you can be, the more you will benefit from the experience.   

As much as you don’t want to hear it, there is a very high chance the test will find vulnerabilities.  While it would be great if applications didn’t have vulnerabilities, it is fairly rare to find them.  Use this experience to learn and train on security issues. Take the feedback as constructive criticism, not someone attacking you.   Trust me, you want the pen testers to find these flaws before a real attacker does. 

Remember that this is for your benefit.  We as developers also need to stay positive.  The last thing you want to do is challenge the pen testers saying your app is not vulnerable.  The teams that usually do that are the most vulnerable. Say positive and it will be a great learning experience.

Future of ViewStateMac: What We Know

December 12, 2013 by · Comments Off on Future of ViewStateMac: What We Know
Filed under: Development, Security, Testing 

The .Net Web Development and Tools Blog just recently posted some extra information about ASP.Net December 2013 Security Updates (http://blogs.msdn.com/b/webdev/archive/2013/12/10/asp-net-december-2013-security-updates.aspx).

The most interesting thing to me was a note near the bottom of the page that states that the next version of ASP.Net will FORBID setting ViewStateMac=false. That is right.. They will not allow it in the next version. So in short, if you have set it to false, start working out how to work it to true before you update.

So why forbid it? Apparently, there was a Remote Code Execution flaw identified that can be exploited when ViewStateMac is disabled. They don’t include a lot of details as to how to perform said exploit, but that is neither here nor there. It is about time that something was critical enough that they have decided to take this property out of the developer’s hands.

Over the years I have written many posts discussing attacking ASP.Net sites, many of which rely on ViewStateMac being disabled. I have written to Microsoft regarding how EventValidation can be manipulated if ViewStateMac is disabled. The response was that they think developers should be using the secure settings. I guess that is different now that there is remote code execution. We have reached a new level.

So what does ViewStateMac protect? There are three things that I am aware of that it protects (search this site for any of these and you will find articles with much more detail):

  • ViewState – protects this from parameter tampering
  • EventValidation – protects this from parameter tampering
  • ViewStateUserKey – Used to protect requests from CSRF

So why do developers disable ViewStateMac? Great question. I believe that in most cases, it is disabled because the application is deployed in a web farm and when the web.config is not configured properly, an error is thrown. When some developers search for the error, many forums recommend disabling the ViewStateMac to fix the problem. Unfortunately, that is WRONG!!. Here is a Microsoft KB article that explains in detail how to properly configure a system to allow ViewStateMac to be enabled (http://support.microsoft.com/kb/2915218).

Is this a good thing? For developers, yes!. This will definitely help increase the protection for ViewState, EventValidation and CSRF if ViewStateUserKey is set. For Penetration Testers, Yes/No. Yes, because we get to say you are doing a good job in this category. No, because some easy pickings are going to be wiped off the plate.

I think this is a pretty bold move by Microsoft to remove control over this, but I do think it is a good thing. This is an important control in the WebForm ecosystem and all too often mis-understood by developers. This should bring many sites one step closer to being a little more secure when this change rolls out.

ViewStateUserKey: ViewStateMac Relationship

November 26, 2013 by · Comments Off on ViewStateUserKey: ViewStateMac Relationship
Filed under: Development, Security, Testing 

I apologize for the delay as I recently spoke about this at the SANS Pen Test Summit in Washington D.C. but haven’t had a chance to put it into a blog. While I was doing some research for my presentation on hacking ASP.Net applications I came across something very interesting that sort of blew my mind. One of my topics was ViewStateUserKey, which is a feature of .Net to help protect forms from Cross-Site Request Forgery. I have always assumed that by setting this value (it is off by default) that it put a unique key into the view state for the specific user. Viewstate is a client-side storage mechanism that the form uses to help maintain state.

I have a previous post about ViewStateUserKey and how to set it here: http://www.jardinesoftware.net/2013/01/07/asp-net-and-csrf/

While I was doing some testing, I found that my ViewState wasn’t different between users even though I had set the ViewStateUserKey value. Of course it was late at night.. well ok, early morning so I thought maybe I wasn’t setting it right. But I triple checked and it was right. Upon closer inspections, my view state was identical between my two users. I was really confused because as I mentioned, I thought it put a unique value into the view state to make the view state unique.

My Problem… ViewStateMAC was disabled. But wait.. what does ViewStateMAC have to do with ViewStateUserKey? That is what I said. So I started digging in with Reflector to see what was going on. What did I find? The ViewStateUserKey is actually used to modify the ViewStateMac modifier. It doesn’t store a special value in the ViewState.. rather it modifies how the MAC is generated to protect thew ViewState from Parameter Tampering.

So this does work*. If the MAC is different between users, then the ViewState is ultimately different and the attacker’s value is different from the victim’s. When the ViewState is submitted, the MAC’s won’t match which is what we want.

Unfortunately, this means we are relying again on ViewStateMAC being enabled. Don’t get me wrong, I think it should be enabled and this is yet another reason why. Without it, it doesn’t appear that the ViewStateUserKey doesn’t anything. We have been saying for the longest time that to protect against CSRF set the ViewStateUserKey. No one has said it relies on ViewStateMAC though.

To Recap.. Things that rely on ViewStateMAC:

  • ViewState
  • Event Validation
  • ViewStateUserKey

It is important that we understand the framework features as disabling one item could cause a domino effect of other items. Be secure.