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

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.