Converting String to Enum at the Cost of 50 GB: CVE-2020-36620

gowebing

In this article, we’re going to discuss the CVE-2020-36620 vulnerability and see how a NuGet package for converting string to enum can make a C# application vulnerable to DoS attacks.

Enum

Imagine a server application that interacts with a user. In one of the scenarios, the application receives data from the user in a string representation and converts it into enumeration elements (string -> enum).

To convert a string into an enumeration element, we can use standard .NET tools:

  
String colorStr = GetColorFromUser();
if (Enum.TryParse(colorStr, out ConsoleColor parsedColor))
{
  // Process value...
}


Or we can find some NuGet package and try to do the same with it. For example, EnumStringValues.

Since we have a sense of adventure (right?), let’s take the EnumStringValues 4.0.0 package to convert strings. The exclamation point in front of the package version makes it even more intriguing.

Here’s what the code using the package’s API might look like:

  
static void ChangeConsoleColor()
{
  String colorStr = GetColorFromUser();

  if (colorStr.TryParseStringValueToEnum<ConsoleColor>(
        out var parsedColor))
  {
    // Change console color...
  }
  else
  {
    // Error processing
  }
}


Let’s see what's going on here:

Strings are converted, and the application is running. Everything seems good... but there’s one thing. It turns out that the application can behave as follows:

Behave

Oh, wait, we have a package marked with an “exclamation point,” right? Let’s try to figure out why there’s such a high memory consumption. The following code will help us:

  
while (true)
{
  String valueToParse = ....;
  _ = valueToParse.TryParseStringValueToEnum<ConsoleColor>(
        out var parsedValue);
}


Using the library, the code infinitely parses strings—nothing unusual. If valueToParse takes values of string representations of the ConsoleColor elements ("Black", "Red", etc.), the application will behave as expected:

Behavior 2

The issues appear when we write unique strings to valueToParse. For example, as follows:

  
String valueToParse = Guid.NewGuid().ToString();


In this case, the application starts to consume memory uncontrollably:

Consume Memory

Let’s try to figure out what’s going on. Take a look at the TryParseStringValueToEnum<T> method:

  
public static bool 
TryParseStringValueToEnum<TEnumType>(
  this string stringValue, 
  out TEnumType parsedValue) where TEnumType : System.Enum
{
  if (stringValue == null)
  {
    throw new ArgumentNullException(nameof(stringValue), 
                                    "Input string may not be null.");
  }

  var lowerStringValue = stringValue.ToLower();
  if (!Behaviour.UseCaching)
  {
    return TryParseStringValueToEnum_Uncached(lowerStringValue, 
                                              out parsedValue);
  }

  return TryParseStringValueToEnum_ViaCache(lowerStringValue, 
                                            out parsedValue);
}


Well, that’s interesting. It turns out there’s a caching option under the hood — Behavior.UseCaching. Since we didn’t explicitly use the caching option, let’s look at the default value:

  
/// <summary>
/// Controls whether Caching should be used. Defaults to false.
/// </summary>
public static bool UseCaching
{
  get => useCaching;
  set { useCaching = value; if (value) { ResetCaches(); } }
}

private static bool useCaching = true;


If the comment to the property is true, caches are disabled by default. Actually, they are enabled (useCaching — true).

You can already guess what the issue is. However, to be sure, let’s dive deeper into the code.

With the knowledge obtained, we return to the TryParseStringValueToEnum method. Depending on the caching option, one of two methods will be called:

  1. TryParseStringValueToEnum_Uncached
  2. TryParseStringValueToEnum_ViaCache:
  
if (!Behaviour.UseCaching)
{
  return TryParseStringValueToEnum_Uncached(lowerStringValue, 
                                            out parsedValue);
}

return TryParseStringValueToEnum_ViaCache(lowerStringValue, 
                                          out parsedValue);


In this case, the UseCaching property has the true value, the TryParseStringValueToEnum_ViaCache method gets control. You can view its code below, but you don’t have to delve deeper into it—further on, we’re going to analyze the method step by step:

  
/// <remarks>
/// This is a little more complex than one might hope, 
/// because we also need to cache the knowledge 
/// of whether the parse succeeded or not.
/// We're doing that by storing `null`, 
/// if the answer is 'No'. And decoding that, specifically.
/// </remarks>
private static bool 
TryParseStringValueToEnum_ViaCache<TEnumType>(
  string lowerStringValue, out TEnumType parsedValue) where TEnumType 
                                                        : System.Enum
{
  var enumTypeObject = typeof(TEnumType);

  var typeAppropriateDictionary 
    = parsedEnumStringsDictionaryByType.GetOrAdd(
        enumTypeObject, 
        (x) => new ConcurrentDictionary<string, Enum>());

  var cachedValue 
    = typeAppropriateDictionary.GetOrAdd(
        lowerStringValue, 
        (str) =>
        {
          var parseSucceededForDictionary =       
                TryParseStringValueToEnum_Uncached<TEnumType>(
                  lowerStringValue, 
                  out var parsedValueForDictionary);

          return   parseSucceededForDictionary 
                 ? (Enum) parsedValueForDictionary 
                 : null;
        });

  if (cachedValue != null)
  {
    parsedValue = (TEnumType)cachedValue;
    return true;
  }
  else
  {
    parsedValue = default(TEnumType);
    return false;
  }
}


Let’s analyze what happens in the method:

  
var enumTypeObject = typeof(TEnumType);

var typeAppropriateDictionary 
  = parsedEnumStringsDictionaryByType.GetOrAdd(
      enumTypeObject, 
      (x) => new ConcurrentDictionary<string, Enum>());


In the parsedEnumStringsDictionaryByType dictionary:

So, we get the following cache scheme: 

Cache <Enumeration type -> Cache <Source string -> Parsing result>>

parsedEnumStringsDictionaryByType is a static field:

  
private static 
ConcurrentDictionary<Type, ConcurrentDictionary<string, Enum>> 
parsedEnumStringsDictionaryByType;


Thus, typeAppropriateDictionary stores a reference to the cache of values for the enumeration type that we are working with (enumTypeObject).

Then the code parses the input string and saves the result in the typeAppropriateDictionary:

  
var cachedValue 
  = typeAppropriateDictionary.GetOrAdd(lowerStringValue, (str) =>
    {
      var parseSucceededForDictionary 
        = TryParseStringValueToEnum_Uncached<TEnumType>(
            lowerStringValue, 
            out var parsedValueForDictionary);

      return   parseSucceededForDictionary 
             ? (Enum) parsedValueForDictionary 
             : null;
    });


In the end, the method returns the success flag and writes the resulting value to the out parameter:

  
if (cachedValue != null)
{
  parsedValue = (TEnumType)cachedValue;
  return true;
}
else
{
  parsedValue = default(TEnumType);
  return false;
}


The key problem is described in the method’s comment: 

This is a little more complex than one might hope, because we also need to cache the knowledge of whether the parse succeeded or not. We’re doing that by storing null, if the answer is “No.” And decoding that, specifically.

Even if the input string could not be parsed, it will still be saved to the typeAppropriateDictionary cache: the null value will be written as the result of parsing. Since typeAppropriateDictionary is a reference from the parsedEnumStringsDictionaryByType dictionary stored statically; objects exist between method calls (this makes sense because they are caches). 

So, here’s what happens: If attackers can send unique strings (that are parsed with the help of the library’s API) to the application, they have an opportunity to “spam” the cache with all the consequences it implies. 

Cache

The unique string parsing makes the typeAppropriateDictionary dictionary bigger. The debugger confirms that the cache is "spammed":

Cash Spam

Additional Information

So, we’ve just discussed the CVE-2020-36620 vulnerability. Here’s some additional information:

The fix is simple—the parsing of input values was removed (the commit). 

Previously, the typeAppropriateDictionary was filled in as data was obtained:

Now, typeAppropriateDictionary is filled in advance. The dictionary initially stores the relationships of string representations of enumeration elements to the actual values:

Values

Input values are not written to the dictionary—there’s only an attempt to extract them:

  
if (typeAppropriateDictionary.TryGetValue(lowerStringValue, 
                                          out var cachedValue))
  ....


This fix made the cache no longer vulnerable to clogging with unique strings. 

The library 4.0.1 already includes the fix, but the corresponding NuGet package is marked as vulnerable. Apparently, the information is taken from the GitHub Advisory. It states that 4.0.2 is the secure version. However, the same entry contains links to data from NVD and vuldb. It indicates the package has been secured since the 4.0.1 version, not 4.0.2. So, there’s some confusion.

Here’s another interesting thing: the vulnerability was closed at the end of May 2019, and information about it appeared in the databases 3.5 years later—at the end of December 2022.

As long as the information about the vulnerability is not recorded in public databases, some tools will not be able to issue warnings about a security defect in the package. On one hand, such a delay is understandable—the project has three forks and sixteen stars, it can be classified as “personal.” On the other hand, the project has 200K package downloads in total—that’s a significant number.

Conclusion

At this point, we have finished the review of the CVE-2020-36620 vulnerability. I hope you enjoyed this article and have taken away some helpful information in case you come across the CVE-2020-36620 vulnerability in the future. Feel free to like and share this article. 

 

 

 

 

Top