Our shop has some generalized error-logging code which, for web pages, includes logging of request properties. This error logger has been stable for years. Lately we've gotten a couple of errors where the user's form input couldn't be processed, and then the logger also bombed when trying to examine the form fields. It logged Request.Headers successfully just before attempting Request.Form. Leaving out some formatting stuff that probably isn't relevant, it basically boils down to:
try
{
foreach (string key in HttpContext.Current.Request.Form.Keys)
{
if (IsPrivateField(key.ToLower()))
AppendToLog(key + ": " + Regex.Replace(HttpContext.Current.Request.Form[key], @"[a-zA-Z0-9]", "*"));
else
AppendToLog(key + ": " + HttpContext.Current.Request.Form[key]);
}
}
catch (Exception ex)
{
AppendToLog("Error: " + ex.Message.ToString());
}
How can there be a null reference exception when accessing Form[key] where key came from the Form.Keys collection? Or alternately, how could Form.Keys include a null value, or be null itself? If you tell me it's impossible, I can post the more complete version to see if I missed anything that adds vulerability to null dereferencing.
We've also had incidents lately where form fields that should have been validated are coming up empty. Maybe we've got some kind of generalized gremlin in our form state.
I'm going to bet this is the problem:
foreach (string key in HttpContext.Current.Request.Form.Keys)
{
if (IsPrivateField( key.ToLower() )) // <-- this line here
It is possible that Request.Form.Keys contains a null key value, so attempting to dereference it to convert it to lowercase here would cause the NRE.
BTW, use ToUpperInvariant() instead of ToUpper, ToLower or ToLowerInvariant to normalize strings owing to the casing rules in a number of different languages, see this article for why: https://msdn.microsoft.com/en-us/library/bb386042.aspx