TeamMentor / Master

TEAM Mentor 3.x Released Code
16 stars 17 forks source link

'Xml Injection Attack' article is DoSing TM (on WikiCreole parsing) #870

Closed DinisCruz closed 10 years ago

DinisCruz commented 10 years ago

while running the script that you can see at https://github.com/TeamMentor/Master/issues/833#issuecomment-51229485 I noticed that there was one article that was 'hanging' the UI. Initially I thought this was just an IE/WatiN issue, but after close look, I was able to pin it down to the

This is what the article view (http://127.0.0.1:32768/article/ae392dbb-fdb4-443f-9d17-78240b4acc95) looks like this (this view uses the Javascript Wiki Creole parser)

image

This is what the html view (http://127.0.0.1:32768/html/ae392dbb-fdb4-443f-9d17-78240b4acc95) looks like this (this view uses the .NET Wiki Creole parser)

image

and eventually (after a good number of minutes) it does this:

image

The end result is

image

... with the same happening if this article is viewed on the main TM UI

image

Editing the article is fine:

image

So there must be something on this source file that WikiCreole doesn't like:

=Applies To

Any application that writes user input to an XML file on a server.

=Description

In an XML injection attack, an attacker inserts malicious data into XML which resides on the server. Depending on the type of XML parser used (SAX or DOM), this malicious data could be used to either overwrite the values in previous nodes or cause the XML parser to consume an excessive amount of memory on the server.

=Impact

    *Modification of previous node values in the XML.
    *Denial of service on the server.
    *Exposure of the entire XML document to unauthorized users.
    *Addition of new nodes or even entire documents.
    *Ability to modify or remove data that should not be accessed.

=Vulnerabilities

    *Dynamic XML generation using untrusted input.
    *Incorrectly defined schema.
    *Lack of schema validation.
    *Failure to validate input and properly encode any dangerous meta characters according to the context.

=Countermeasures

    *When loading an XML file on the server, validate it against a defined XSD.
    *Perform context-sensitive encoding of untrusted input: As with encoding data to be output in HTML, take a whitelist approach. For each context into which data will be inserted (element, attribute value, etc.), construct a whitelist set of known-safe characters. Check the data to be added against the whitelist, and encode any characters that are not on the whitelist. Use CDATA sections to further ensure that the parser ignores special characters. Ideally, accept only non-XML values from untrusted sources. If an XML blob must be accepted from an untrusted source, manually reassemble it from values parsed out and encoded after checking the received blob against an XSD.
    *Validate untrusted input against an inclusion list before use: For example, use a RegEx pattern, primitive type casting, a domain constraint, or another validator. See the Additional Resources section below for more information on data validation.
    *Trim XML documents to include only the data that is necessary to the application.
    *Use XML processing pipelines to make the flow of data clear and threats easier to identify.
    *Remove DOCTYPE element from user input before creating XML objects.

=Example

Let us assume that a valid node in our server side XML looks like this:

{{{
<userrecord>

<uniqueid>5</uniqueid>   
<name>Gandalf</name>   
<email>Gandalf@MiddleEarth.com</email>   
<address>One Middle Earth Way, ME</address>   
<zipcode>10000</zipcode>   
<phonenumber>123-456-7890</phonenumber>

</userrecord>
}}}

The {{{UniqueID}}} field here is assigned by the server. Now, if web server accepts input from the user and stores it in the XML file, what is the outcome of the parser when a user enters the following data for the {{{Email}}} field:

{{{
Gandalf@MiddleEarth.com<uniqueid>0</uniqueid><name>Gandalf</name><email>Gandalf@MiddleEarth.com
}}}

The resulting XML on the server will look like this:

{{{
<userrecord>

<uniqueid>5</uniqueid>   
<name>Gandalf</name>   
<email>Gandalf@MiddleEarth.com</email>
<uniqueid>0</uniqueid>               
<name>Gandalf</name>
<email>Gandalf@MiddleEarth.com</email>   
<address>One Middle Earth Way, ME</address>  
<zipcode>10000</zipcode>   
<phonenumber>123-456-7890</phonenumber>

</userrecord>
}}}

We now have two fields and values for {{{UniqueID, Name and Email}}}. The outcome of parsing such data will depend upon the type of XML parser used:

    If a {{{SAX}}} parser is used, it will report the {{{UniqueID}}} for this node to be 0, so an attacker would have essentially overwritten the field value.
    If a {{{DOM}}} parser is used, it will report an error. However, the {{{DOM}}} parser can be abused by inserting additional nodes in the XML while still matching the schema, which will lead to excessive memory consumption by the parser and could potentially result in a denial of service.

Also note that this is a simple example. It is possible to insert an entire new record, among other issues. To fix the example above, we can escape the markup delimiters to prevent the user input from being interpreted, replacing & with &, < with <, and so on, as described in the Countermeasures section. This will prevent the user data from being interpreted in this case. In general, though, we don't know exactly what the input might look like, so the best plan is to validate against a whitelist of acceptable characters and do type checking as much as possible. Where type checking is not possible, consider using {{{CDATA}}} to enforce that the string be treated strictly as data. For example:

{{{
$evil_input = "Gandalf@MiddleEarth.com</email>   
<uniqueid>0</uniqueid>
<name>Gandalf</name>
<email>Gandalf@MiddleEarth.com";$data = "<!--[CDATA[" + $evil_input + "]]-->";
}}}

After we insert {{{$data}}} into the {{{email}}} field above, the result will be the following acceptable XML:

{{{
<userrecord>

<uniqueid>5</uniqueid>  
<name>Gandalf</name>  
<email><!--[CDATA[Gandalf@MiddleEarth.com</Email--><uniqueid>0</uniqueid>            
<name>Gandalf</name>
<email>Gandalf@MiddleEarth.com]]></email>  
<address>One Middle Earth Way, ME</address>  
<zipcode>10000</zipcode>  
<phonenumber>123-456-7890</phonenumber>
</email>

</userrecord>
}}}

{{{CDATA}}} is very useful when used correctly but it has the same generic problem as we had before. An attacker can create a malicious string that uses {{{"]]>"}}} to end the
{{{CDATA}}} and insert malicious XML. Therefore, wherever we use {{{CDATA}}} we must also remove or escape {{{]]>}}} from the input string—at a certain point, it's easier to simply give up and encode everything by whitelist.

=Additional Resources

    *For more information on the XML Processing Model, see http://www.w3.org/XML/Processing/
    *For more information on Managing XML Datasets for Security, see: http://www-128.ibm.com/developerworks/xml/library/x-think37/index.html
    *For more information on Data Validation, see: http://www.owasp.org/index.php/Data_Validation
    *For more information on Configuring SAX Parsers for Security, see: http://www-128.ibm.com/developerworks/xml/library/x-tipcfsx.html
    *For more information on XML CDATA, see: http://www.w3schools.com/xml/xml_cdata.asp

=Related Items

    *[[1508d763-03f1-4b43-bf8e-ad7cf8edebd7|Attack: Xpath-XQuery Attack]]
    *[[b1757bda-0a0d-48e7-b101-3c4088e41878|Attack: SQL Injection Attack]]
    *[[8482159c-5ec2-4b89-9c65-9af765030ff5|Attack: Information Disclosure Attack]]
    *[[9d60faca-687b-4c69-91ba-f5712af0fd02|Attack: Server-Side Code Injection Attack]]
    *[[e4a899ec-9301-4751-ae07-69a265336d8b|Attack: AJAX Injection Attack]]
    *[[61f0d74e-d9b6-4e27-9e12-da65baff83fd|Attack: LDAP Injection Attack]]
    *[[1408d3c3-7fc1-4ff0-910e-cdf0e191b669|Attack: Client-side Validation Attack]]

Roman, I'm marking this as P0, since it is a nasty bug which brings TM down to its knees

DinisCruz commented 10 years ago

Here is is a script that confirms this timeout (note-to-self: run this on all TM Articles)

tmProxy.admin_Assert();

tmProxy = nUnitTests_Cassini.tmProxy();

Func<Panel> getRightPanel = ()=>
    {
        if (ie.HostControl.parent().parent().controls<Panel>(true).size() != 4)
        return ie.HostControl.insert_Right();

        return ie.HostControl.parent().parent().controls<Panel>(true).fourth().clear();
    };
var panel = getRightPanel();
//var textArea = panel.clear().add_TextArea();
var dataGridView = panel.add_DataGridView().add_Columns("#","Id", "Title", "Status", "Html Render time", "Article Size", "Article Render Time");
var index = 0;

foreach(var article in tmProxy.articles().take(550))
{
    var article_Id    = article.Metadata.Id.str();
    var article_Title = article.Metadata.Title;
    var start          = DateTime.Now;

    ie.div("Title").innerHtml("");

    //ie.open_ASync(ieTeamMentor.siteUri.append("article/".append(article_Id)).str());          
    ie.open_ASync(ieTeamMentor.siteUri.append("html/".append(article_Id)).str());           

    index++;
    var title = "";
    try
    {   
        for(int i=0; i< 10; i++)                    // give it about 1 sec
        {       
//          title = ie.div("Title").innerHtml();
            title = ie.div("title").innerHtml();
            if(title == article_Title)
              break;
            else
                100.sleep();
        }
        var article_Time   = start.duration_To_Now();   

        if (title != article_Title)
        {
            dataGridView.add_Row(index,article_Id, article_Title,"TIMEOUT", "", 0, article_Time);   
        //  return "Problem getting {0} - {1} != {2}".format(article_Id, article_Title, title);
        }
        else
        {
                var article_Render = ie.div("guidanceItem").innerHtml();    
                var article_Size   = article_Render.size().str(); 

                dataGridView.add_Row(index,article_Id, article_Title, "OK", "", article_Size, article_Time);    
        }
    }
    catch(Exception ex)
    {
        dataGridView.add_Row(index, article_Id, article_Title, "ERROR: " + ex.Message, "", "0", "");    
    }
}

//O2Ref:TeamMentor.UnitTests.Cassini

Note top row (# = 305) of the execution results below:

image

DinisCruz commented 10 years ago

other articles that don't load on WikiCreole:

Html 5 Library:

michaelhidalgo commented 10 years ago

This is a great and interesting finding Dinis. I was able to reproduce it in my environment and I got the same results, in fact I ran their (Algorithm.WikiCreole) unit test against this article and it is still trying to parse it. It lead us to always test a bit better third party libraries with Unit Test, something similar to what needs to be done for regular expressions.

In fact, I got a OutofMemory Exception.

Test method Tests.Algorim.CreoleWiki.CreoleParserTests.Parser_Si threw exception: System.OutOfMemoryException: Exception of type 'System.OutOfMemoryException' was thrown

romichg commented 10 years ago

So how do we fix it? Remove the article? Delete each line one at a time to see what fixes it? On Aug 6, 2014 9:35 AM, "Michael Hidalgo" notifications@github.com wrote:

This is a great and interesting finding Dinis. I was able to reproduce it in my environment and I got the same results, in fact I ran their (Algorithm.WikiCreole) unit test against this article and it is still trying to parse it. It lead us to always test a bit better third party libraries with Unit Test, something similar to what needs to be done for regular expressions.

In fact, I got a OutofMemory Exception.

Test method Tests.Algorim.CreoleWiki.CreoleParserTests.Parser_Si threw exception: System.OutOfMemoryException: Exception of type 'System.OutOfMemoryException' was thrown

— Reply to this email directly or view it on GitHub https://github.com/TeamMentor/Master/issues/870#issuecomment-51334628.

DinisCruz commented 10 years ago

This is fixed with https://github.com/TeamMentor/Dev/commit/9e914c112700d722b888cadafcf532e881a3a7ad

Like the comment says, it is a hack (I fix the wiki text only for WikiCreole), but it seems to do the trick.

I looked at reverting the code to how 3.4 used to do it, but that would introduce too many code changes: both html rendering (needed for checkmarx/others) and main UI Preview used the same WebService Method

DinisCruz commented 10 years ago

Sorry, the comment above is wrong (it was meant for #833)

That said, that fix is correct for this issue, since it introduces a change where there is a Thread created for rendering the HTML (which is given 2sec to complete)

DinisCruz commented 10 years ago

I just added a 'friendly error message'

image

romichg commented 10 years ago

It seems like the friendly error message does not always work. I tried it with the All Password Fields Are Masked : https://localhost:3187/html/1513a133-0a53-46b5-b0b1-79b6dcb52ca2

And it just stays that way. No unusual CPU activity

image

Original offending article - XML Injection attack in the Vulnerabilities library - ae392dbb-fdb4-443f-9d17-78240b4acc95. The friendly error message works fine.

image

The interesting part is when I access the article directly, by using this link:

http://127.0.0.1:12120/article/ae392dbb-fdb4-443f-9d17-78240b4acc95

It opens up pretty much instantaneously.

image

However, when I double click on the article from the article list, it takes about a second or two to load. And there is a noticeable spike in the CPU at that time. image

romichg commented 10 years ago

I am not really sure how much more time we should spend on this one. Check the friendly message in my first example, see if there is an easy reason you can pinpoint why it doesn't work. Otherwise I think lets leave it for now. This will get fixed eventually when all the articles move to markdown.

romichg commented 10 years ago

I am marking it as P3 since it is no longer a security risk, with the implemented timeouts.

DinisCruz commented 10 years ago

The problem with https://tm-dinis.azurewebsites.net/article/1513a133-0a53-46b5-b0b1-79b6dcb52ca2 is that it throws an internal Algorim.CreoleWiki.CreoleParser error

see image

You can replicate the problem (empty result) here:

https://tm-dinis.azurewebsites.net/html/1513a133-0a53-46b5-b0b1-79b6dcb52ca2

image

DinisCruz commented 10 years ago

Fixed with https://github.com/TeamMentor/Dev/commit/22cb6c20249174c9bc264fbeaf443e04aee77597

here it the fix on the dev box

image

Here is the fix on the test Azure server

image

Error message also shows the Markup that failed to parse:

image

DinisCruz commented 10 years ago

Btw, Michael, here is a simple script that I used to quickly try the C# Wiki transformation

var creoleParser = new CreoleParser();
var topPanel = panel.clear().add_Panel(); 
var wikiText = topPanel.add_TextBox(true);
var html     = topPanel.insert_Right().add_TextArea();
var browser  = html.insert_Below().add_WebBrowser(); 

wikiText.onTextChange(
    text =>{                
                var parsedWikiText = creoleParser.Parse(text);
                html.set_Text(parsedWikiText);
                O2Thread.mtaThread(()=>browser.set_Html(parsedWikiText));
           });

//using Algorim.CreoleWiki;
//O2Ref:E:\TeamMentor\TM_Dev Repos\TM_Dev\Source_Code\packages\Algorim.CreoleWiki.1.0.4335.37128\lib\net40\Algorim.CreoleWiki.dll

Looks like this when executed from the O2's C# REPL environment:

image

romichg commented 10 years ago

Great, this is working fine. Michael, can you run this script on all the articles to see which ones will cause this to happen.

romichg commented 10 years ago

After Michael's review all but one article (5.Article: XML Injection Attack) had been converted to markdown. The last article issue is tracked at: https://github.com/TeamMentor/Master/issues/882 Closing.


After doing a review, I found that 5 out of 875 wiki text articles have the problem with the server side Wiki Creole library :

1.Article:All Password Fields Are Masked , Id 1513a133-0a53-46b5-b0b1-79b6dcb52ca2 , Technology Web Application URL: https://tmqa-dev.teammentor.net/html/1513a133-0a53-46b5-b0b1-79b6dcb52ca2 2.Article: Use Mapping Values When Redirecting , Id 39a0a4e5-080d-444a-9789-b406a0aef602 , Technology Web Application URL: https://tmqa-dev.teammentor.net/html/39a0a4e5-080d-444a-9789-b406a0aef602 3.Article: Mapping Values Are Used when Redirecting , Id a5dee3ea-30a0-433c-bbb3-22444d35c5a2 , Technology Web Application URL: https://tmqa-dev.teammentor.net/html/a5dee3ea-30a0-433c-bbb3-22444d35c5a2 4.Article: Mask All Password Fields , Id f24d1109-4dd5-4360-867f-2be446baf6c9 , Technology Web Application URL :https://tmqa-dev.teammentor.net/html/f24d1109-4dd5-4360-867f-2be446baf6c9 5.Article: XML Injection Attack , Id ae392dbb-fdb4-443f-9d17-78240b4acc95 , Technology Any URL :https://tmqa-dev.teammentor.net/html/ae392dbb-fdb4-443f-9d17-78240b4acc95