Our website uses cookies to enhance your browsing experience.
Your message has been sent. We will email you at
If you do not see the email in your inbox, please check if it is filtered to one of the following folders:
Webinar: C++ semantics - 06.11
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.
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 how the code using 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, the application is running. Everything seems good... but there's one thing. It turns out that the application can behave as follows:
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 ConsoleColor elements ("Black", "Red", etc.), the application will behave as expected:
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.
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, well, well, interesting. It turns out that 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's the issue. 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 — TryParseStringValueToEnum_Uncached or 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, the key is the type of enumeration that is used, and the value is the cache of strings and the results of their parsing.
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 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 consequences that it implies.
The unique string parsing makes the typeAppropriateDictionary dictionary bigger. The debugger confirms that the cache is "spammed":
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, 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:
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 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 that 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 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 the one hand, such a delay is understandable — the project has 3 forks and 16 stars, it can be classified as "personal". On the other hand, the project has 200K package downloads in total — that's a significant number.
**
At that point we finish the review of the CVE-2020-36620 vulnerability. If you enjoyed it, I invite you to look through a couple of more similar notes:
1. Why does my app send network requests when I open an SVG file? The article describes how a NuGet package used for working with images can make an application vulnerable to XXE attacks.
2. The story of how PVS-Studio found an error in the library used in... PVS-Studio. A short story describing how we found a bug in the source code of a library that we use in our own product.
P.S. I also post links to my publications on Twitter and LinkedIn — perhaps it will be convenient for someone to follow me there.
0
0
0
126
1
0