Anatomy of a Sitecore bug (and some things that all of us developers must learn)

Recently Sitecore published the following critical report https://kb.sitecore.net/articles/039942 including a fix.

 

Sitecore has rightly marked it as critical as the result of the bug actually is that all contents of the website can be downloaded by simply specifying a custom crafted url. I will not post the format of the url but will only describe why some of the checks that where done in the code failed to ensure that this could not happen. In order to make this exploit work 4 problems exists and used together they result in this critical issue.

 

Problem 1

The first problem (which is the usual case with many bugs of the same kind) is that url’s are not treated with proper respect.

Whenever you ask for

System.Web.HttpContext.Current.RawUrl

Then you get exactly what you ask for. You get the url from the browser including everything in it (except for any #-element prepended to the url, which is a browser only element).

This usually is not a problem but when you consider that an url can contain the following characters []~:// in the url part you might want to reconsider how you use the raw url.

 

Solution

The solution is to never trust the input you get in an URL-request and to further help you, you should never parse url strings on your own but instead you should rely on the .NET framework to retrieve the various parts of the url depending on what you need to do. This is no garantee to be flawless but it will definitely increase your chances that someone else has handled that edge case scenario, that you forgot to think of.

 

Problem 2

The second problem is a more global issue, but it falls along the same line as problem 1. You should never make assumptions about what your method input is supposed to be like and then just continue if something is not as expected.

This means that if you have a method that expects an input string of “somevalue/someothervalue”, and you want to trim the part of “somevalue” and remove that from the beginning of the string. Then you need to consider how to do it. Do you just call

inputstring.Replace(“somevalue”, “”)

or

inputstring.SubString(inputstring.IndexOf(“somevalue”))

or

inputstring.Split(new string[] { "somevalue"}, StringSplitOptions.RemoveEmptyEntries)[0]

or

something completely different.

No matter the path you choose you need to be aware of the sideeffects of the methods. What happens if the string is not at the beginning or it appears twice, then how will your logic react. So you need to make sure that your assumptions about your input is correct.

 

Solution

If you do not fully trust the input for you method you need to test it and make sure that all the assumptions that you have about your input is correct. In this case test your input to make sure it matches your assumptions, and then fail if your assumptions are not met or otherwise act accordingly.

 

Problem 3

The third problem is related to making assumptions about the world around you based on only partial knowledge and then trying to create your own logic to ensure that your assumptions are correct.

As an example if you have a method that takes an url-string as input, and you want to see if it is an external absolute url or relative url. Then first of all you need to watch out for problem number 2 and make sure that your assumptions about your input is correct.

Secondly you need to find out which logic you which to use to figure out what you want to know. One way to test if the url is external could be by testing if it contains the string ://, this would work for http://example.com and for /samplepage.html providing the correct result. But what happens if the url is /samplepage.html?://, then your rule would provide the wrong answer. So that means that if you want a valid result at all times you need to introduce more rules.

 

Solution

The solution in this case is again to rely on pre-existing methods as much as possible. If the .NET framework contains some logic that can help you out, then you should use it. In this case something simple as

new Uri(path,  UriKind.RelativeOrAbsolute).IsAbsoluteUri
could be used. That way if a bug is found in the framework there is a great chance the it will be fixed in the next framework update.

 

Problem 4

The fourth problem is related to making generic methods too generic and thereby exposing too much. In the case of of this Sitecore bug, the problem is well-known as directory-traversal. It simply allows you to use a generic method to navigate out scope and for websites out of scope means accessing files that you are not supposed to access.

 

Solution

Be very careful when creating generic methods and ensure that even though they are generic, you need to enforce some limits on scope.

 

 

The Fix

Eventually what has been fixed in the Sitecore update is problem number 4, where scopes have been introduced to ensure that only a very specific set of files can be retrieved and thus limiting the bug. This in turn makes all the other problems irrelevant in this particular scenario as the solution to problem 4 is a final block and thus mitigates the errors introduced by the other problems.

But this is a typical bug where several methods that are being misused in combination can result in a serious issue. Therefore you must always think about every component that you code, because if any of theses bugs had not existed there would not have been a problem and all parts of the code are relevant in a security perspective to act as expected.

Iterating items in a list - for vs. while vs. foreach

When iterating over the items in list in C# you have at least 3 options as to how you want to do it. This blog post is simply a small journey into exploring the 3 options for, foreach and while seen from a performance perspective.

 

For this journey I will take a look at the following very simple console application

using System;
using System.Diagnostics;
using System.Linq;

namespace ConsoleApplication1
{
    internal class Program
    {
        private static void Main(string[] args)
        {
            var iterations = 10000000;
            var test = 0;

            for (var a = 0; a < 50; a++)
            {
                var list1 = Enumerable.Range(0, iterations).ToList();
                var list2 = Enumerable.Range(0, iterations).ToList();
                var list3 = Enumerable.Range(0, iterations).ToList();


                var st = new Stopwatch();
                st.Start();
                for (var i = 0; i < list3.Count; i++)
                    test = list3[i];
                st.Stop();

                var st2 = new Stopwatch();
                st2.Start();
                foreach (var i in list2)
                    test = i;
                st2.Stop();


                var st3 = new Stopwatch();
                var iterationcount = iterations;
                st3.Start();
                while (iterationcount-- > 0)
                    test = list1[iterationcount];
                st3.Stop();

                Console.WriteLine("1: [{0}] 2: [{1}] 3: [{2}]", st.ElapsedMilliseconds,
                    st2.ElapsedMilliseconds, st3.ElapsedMilliseconds);
            }
        }
    }
}

This is a very simple test case just to prove a point

I have a loop where I do 50 test runs and for each test run for each type of loop I create a sample list of 10.000.000 integers and then I simply test each type of loop to see which loop type will give me the fastest access to all my numbers again.

 

Before doing the test I would consider them more or less equal and I would actual expect the foreach loop to be the fastest because it does not have to continuously index the list. But having run the code multiple times (and even having rearranged the order of the loops), the result always come out similar and looking somewhat like this

Loop performance

 

So most of the time the for-loop is the slowest. The foreach-loop is in second and then the while-loop is actually the fastest.

 

Off course your mileage may vary depending on what you are trying to do, but if you need to squeeze out the last milliseconds in your app, you can consider which kind of loops that you are using.

Øredev - a conference with an attitude

This Wednesday I showed up for my first Øredev conference in Malmö.

I was prepared for a technical conference where I was supposed to learn some new tricks to improve my development workflow and make me more productive and efficient.

What I got from the conference though was as expected but also much, much more.

The conference was set for 3 days and throughout the conference there had been set a scene and there was a red line for the days. The theme this year was Computer Power. Within that theme was both the power in the computers and devices that we have, but also the power that we as developers have utilizing computers.

Going through the days the talk went from

- The state of world today

- The possibilities of the future

- The actions we can take to ensure that we get the future that we want

The focus was that if we, the developers, do not take action now, and develop in an ethical manner, the future might not look so bright.

Furthermore we need to start voicing our concerns for the current politics and ensure that the government and corporations start acting ethically and respectfully and ensure our privacy.

So now is the time to start making some noise.

Using Sharepoint AuthoringContainer on pages with Url Extension

This blog post is based on a real life issue, that we experienced on our site once.

 

In the header of out Sharepoint masterpage we had inserted 2 AuthoringContainer controls with a plan for adding two different stylesheets to the page. One stylesheet used for Reader and another for Editors. This seemed pretty straightforward to achieve using the DisplayAudience property on the AuthonringContainer.

It can have the 2 values

  • AuthorsOnly
  • ReadersOnly

So adding two authoring containers and setting that property to match each scenario seemed like the right way to go, and everything worked as expected.

 

 

The one day we received information that the page looked strange in certain scenarios for some pages. After some debugging we found a common pattern for the urls, as the urls that not displaying correctly looked like this

http://domain/site/pages/default.aspx/moreinfo

http://domain/site2/pages/default.aspx/info

 

The patthern for these urls is that they are in a format the ends in /filename.fileextension/urlextension. This last part of the url can be retrieved in .NET using HttpRequest.PathInfo and as such it is a known format. The problem with this format is that whenever something is added to the url like this Sharepoint is not able to properly set SPContext.Current.ListItem and by default the AuthoringContainer is not displayed if there is no current listitem.

 

The solution:

Very simply you can set the property RequireListItem to false and the it works as expected, so the tag looks like this

<PublishingWebControls:AuthoringContainer ID="AuthoringContainer1" DisplayAudience="ReadersOnly" RequireListItem="false"
        runat="server">

A matter of scopes (method paramaters versus class properties)

Defining variable names must be done carefully sometimes to prevent logical errors in your code. Just to provide and example of where things can go wrong you can see the following snippet.

 

public class TestClass
    {
        public int Variable { get { return -1; } }
        private int _variable1;
        private int _variable2;
        public TestClass(int Variable)
        {
            _variable1 = Variable;
            _variable2 = this.Variable;
        }

        public override string ToString()
        {
            return "Value of variable1: \{_variable1}\{Environment.NewLine}Value of variable2: \{_variable2}";
        }
    }

 

This example is pretty straightforward and easy to correct, but can be harder to detect in a larger code base.

 

But the lesson is to be careful when naming your variables to ensure that you know exactly what variables are used at which times.

In the above example the method parameter is more locally scoped and is thus prioritized rather then the class property. And in order to use the class property you need in this case to use the "this" keyword.

Sjov oversættelse - Sharepoint 2007

Jeg har fuld forståelse for at det kan være svært at oversætte software og de ledetekster der er i en softwareløsning. Men det er alligevel meget sjovt når man støder på nogle mærkelige tekster.

 

F.eks. denne dialog som dukker op i Sharepoint 2007 når jeg prøver at slette et dokument fra et dokumentbibliotek.

 

Men svaret er "Nej, jeg vil gerne slette dem,  ikke genbruge dem" :-)

Funny thing – Sharepoint 2010 - NotImplementedException

During a debugging session I was just poking around some Sharepoint dll's using Reflector.

And then I stumbled upon this little snippet. I must say, I didn't expect to find this exception being thrown in a released Microsoft product.

It does implement an interface, so I presume it must make sense somehow, I was just surprised to see it.

 

 

Happy coding

Variable scopes in C# - Beware of naming similiarities

Just the other day I came across a sample of code with some similar named variables, where the flow did not work probably. It turned out to be an issue of some locale variables which were named the same as some global variables.

This made me think some about it and do some testing about scoping, and I created this small snippet to illustrate what can be done and what cannot be done

using System;
namespace ScopeTest
{
    class Program
    {
        static void Main(string[] args)
        { new Program().Debug(); }
 
        int temp = 1;
        void Debug()
        {
            int temp = 2;
            Console.WriteLine(temp); //Prints 2
            Console.WriteLine(this.temp); //Prints 1
            //Action a = delegate() //Will not compile
            //{
            //    int temp = 3;
            //    Console.WriteLine(temp);
            //};
            //a();
        }
    }
}

 

This is just something you need to know and be aware of otherwise you might experience strange unexpected issues.

Updating Sharepoint menu to display itemcount in menu

I just though I would share this small snippet I created which can be added to a Sharepoint page on a website, and then when called it will iterate through the leftmenu and for links to folders/list in the current site it will add a counter indicating the number of items in the given list.

It is very simple and naïve in its implementation, but I think the concept is pretty neat

 

function ShowFolderCount() {
    var ctx = SP.ClientContext.get_current();
    // point  to get all the lists  
    var myLists = ctx.get_web().get_lists();
    // Load all lists 
    var listinfoarray = ctx.loadQuery(myLists, "Include(Title, ItemCount, RootFolder)");
    // async excute 
    ctx.executeQueryAsync(function () {
        var listinfo = new Array();
        // iterate to all items 
        for (var i = 0; i < listinfoarray.length; i++) {
            var listdata = new Object();
            var currentItem = listinfoarray[i];
            listdata.title = currentItem.get_title();
            listdata.count = currentItem.get_itemCount();
            listdata.url = currentItem.get_rootFolder().get_serverRelativeUrl();
            listinfo.push(listdata);
        }
        var submenus = jQuery(".ms-quickLaunch .menu .root ul.static");
        submenus.each(function () {
            var menu = jQuery(this);
            var links = menu.find("a");
            links.each(function () {

                try{
                    var link = jQuery(this);
                    var url = unescape(link.attr("href"));
                    for (var t = 0; t < listinfo.length; t++) {
                        var c = listinfo[t];
                        if (url && url.startsWith(c.url)) {
                            link.find("span span").text(c.title + " (" + c.count + ")");
                            break;
                        }
                    }
                }
                catch(ex)
                {}
            }
            );
        }
        );

    }, function () { });
}