1

For security and consistency I would like to test on post-back if a field is missing? In Java (servlets in particular) when we perform a request.getParameter(key) the result is either a String value or otherwise NULL if the field is missing. In MVC I've created a custom validation that I call "ThrowOnNull". The behavior I'm trying to capture is: If an intended field is missing (null) I want to throw, otherwise return success.

Consider this Custom Validator (that doesn't work):

public class ThrowOnNull: ValidationAttribute
{
   public ThrowOnNull() { }

   protected override ValidationResult IsValid(object value, ValidationContext validationContext)
   {            
       if (value == null)
           throw new Exception(validationContext.MemberName + " field expected, but missing."));            

       return ValidationResult.Success;
   }
}

Is it possible to do what I want here? (this validator doesn't work as expected and it's because the framework is assigning NULL to an empty value [oh dear].)

UPDATE: Per @emodendroket, the following code example now works as expected:

public class ThrowOnMissing: ValidationAttribute
{
    public ThrowOnMissing() { }

    protected override ValidationResult IsValid(object value, ValidationContext validationContext)
    {
        if (!HttpContext.Current.Request.Form.AllKeys.Contains(validationContext.MemberName))
            throw new Exception(validationContext.MemberName + " field expected, but missing.");

        return ValidationResult.Success;
    }
}

EDIT: I've cleaned up the question and example code significantly to make it all much clearer, hope it helps.

Improve this question
2
  • I fail to understand how the answer you accepted solves the problem you are so adamant about.... Commented Sep 4, 2014 at 22:13
  • In short: It solves it because it explains how it's possible to what I want. Which is: Check the raw incoming transmission for consistency though a custom validator. Further: Ensuring the incoming data (fields) are what my application expects. If not, throw, log and redirect. This technique peeks over the framework (and any reliance on it) looking at the raw stream allowing me to provide a preemptive (pre-binding) examination and certification. I welcome a MVC approach if a reasonable one is offered or explained (which hasn't been). Otherwise, I'll peek over the framewk and validate it myself. Commented Sep 4, 2014 at 22:28

4 Answers 4

Reset to default
3

You're missing one important point - when you submit a form, all fields belonging to that form get submitted. If the user doesn't fill them, they're blank, so the validation for null can't really work like this...

You should probably redefine what "missing value" means.

EDIT:
After some discussion it seems you're not really concerned about null values, but about the security (false forms with missing fields and stuff like that). In that case you should simply use the standard - antiforgery token and maybe check the origin of the request. You should be more than fine this way. Checking for missing fields won't help a bit, because attacker can easily send those fields as well.

Over/Under posting is a real concern. Furthermore, if fields aren't submitted (due to a hack or DOS attack of some kind)

Not really.

Overposting is handled by the action method on the controller. It won't accept more parameters than you've specified.

Underposting will be handled pretty much the same way as if you didn't fill the text fields in the form, again a non-issue if you have validated your model correctly.

DDOS attack can't be prevented a believe me, some checking for missing fields won't help a bit if someone has a network powerful enough to cause DDOS. Just look up latest cases on attacks and you'll understand, that if HUGE servers can't withstand that, you certainly can't prevent it like this.

Your data validation shouldn't be too expensive either. It's a web, people don't like to wait too much.

Improve this answer
Sign up to request clarification or add additional context in comments.

27 Comments

Agreed - a required attribute and a well defined, bound view model are the way to go here - if the fields aren't sent, or too many fields are set the binding will throw, preventing "further and possibly expensive validation", and if the field is sent, but is empty the required attribute will catch it (but won't prevent further validation as you probably want to tell the user about all the problems, rather than one at a time...)
but instead simply shows it as a validation error "Field is Required" and what else would you expect from a validation?? If you're having troubles with some hackers, this would prevent maybe some 15yo-wannabe-hackers and even there I have some doubt. If you're concerned about XSS, creating malicious forms or something like that, you should use techniques that were designed for that kind of stuff (using tokens, checking origin of the request if it's the same etc). That kind of stuff. Checking for null won't help a bit.
@JasonCaldwell, and why exactly you refuse to use antiforgery token and checking of the origin of the POST request? I somewhat fail to see what would accomplish if you could find the missing fields. If you use standard security approaches, you're more than fine.
@JasonCaldwell, funny that you think using antiforgery token is super easy to bypass, but you consider checking for missing fields a superior solution. Erik Funkenbusch said pretty much the same, but elaborated on the topic a bit more. Asp.net has many built-in security features and your fear seems to come from java world. .NET is very flexible as well, probably more than you currently see. If you don't agree with us and still want to reinvent the wheel, no problem.
@JasonCaldwell, you laugh at me (what a mature thing of you to do, but OK), yet your solution is to check for a missing field to prevent a hack. Wow, I must admit I'm really stunned from your reasoning. I've updated the answer an hour ago, take it or leave it. This is really not a place for prolonged debates and I think me and Erik have already said enough on this matter.
|
2

If you want your own validator you can look at the dictionary HttpContext.Current.Request.Form. You can then do what you've proposed with this code:

if (!HttpContext.Current.Request.Form.AllKeys.Contains("prop"))
  {
    throw new Exception();
  }
Improve this answer

5 Comments

The problem here is that you won't get to execute this code until after full validation has occurred by the model binder. So your requirement to abort validation is not satisfied by this approach.
@ErikFunkenbusch Well, now that you mention it, you're right about that, since by the time you're inside your action the model is already bound. Putting this inside a custom model binder presumably would work. You could even just have a model binder inheriting from the default and reflect over all the properties with the attribute and throw if any of their keys are missing before then passing to the base method.
You wouldn't need to put this code in a custom model binder, because the binder already is using reflection to go over all the properties and compare them to the properties of the posted objects. And, indeed, your conclusion is exactly what I described in my post.
"The problem here is that you won't get to execute this code until after full validation has occurred" -- is this true even if I'm using a Custom Validator and throwing at that instant?
Cleaned up the question and code examples to make it much clearer and included an example that does work per @emodendroket.
1

I think you're being unreasonably paranoid here about things. Note that I said Unreasonably paranoid, since a little paranoia is a good thing.

First, let us analyze the "threats" and determine the risks. You've presented several arguments:

  1. Over-posting
  2. Under-posting
  3. Validation of empty vs null

The first item is not an issue in MVC if you are using a View Model. You simply can't post information that the view isn't expecting (well, you can, but it's ignored). If you're not using a view model (or not using a properly defined one) then you can use binding attributes to prevent posting of items you don't want bound.

The second, under-posting, if it's truly a concern for your model (99.999% of the time simply treating it as required is more than fine, but ok let's take your argument. The issue here is not the validation attribute, it's the model binder. You simply have to register a custom model binder that looks for missing values in your view model and throws if they are null. This is easily accomplished by reflecting over the bound model and comparing it to the posted values, then throw.

The problem with your RequiredThrowIfNull approach is.. what if it's not required and it's under posted? That's still an error according to your argument.

Finally, validation of empty vs null... you talk about expensive validation... I don't know what kind of expensive validation you could be talking about here, but server side there is noting in attributes that could be considered expensive.

The reason your attribute doesn't work is because validation attributes are called within a try/catch block by the framework already, and if it's null it's the very mechanism that treats it like empty (this mechanism also does things like catching parsing errors when a type is incorrect, such as characters in a datetime field.)

.NET is not Java, even though it largely works similarly... trying to re-implement Java patterns in .NET is going to lead to misery on your part because many basic philosophies are just different.

Even if we accept your argument of wanting to catch errors or be notified of hacking attempts, throwing is the wrong thing to do in most cases. Instead, just log the information from the model binder and continue on as normal. Only throw if you absolutely need to abort the request, and most of the time that simply isn't the case, even if you're being hacked... throwing will just cause the attacker to vary their attack until they no longer get the exception.

Frankly, this is an over-engineered, solution looking for a problem and there are many good .net specific ways of dealing with the REAL issues you are trying to solve.

Improve this answer

7 Comments

I largely disagree. Not enough room here to take you point by point, but as I said to Walther: "there are different forms of validation. There's user input validation and there's system/data error consistency and validation. Each has their place. Consistency ensures that what comes across the wire is what I expect and if it isn't, someone is either screwing w/my site or I have an error somewhere. These kinds of errors aren't for end users and are best dealt with the instant that happen to avoid other possible exploits or problems. While I agree the threat of this sort is low, it does occur"
Furthermore, I understand the distinction between Java and ASP-MVC and I'm not trying to roll one over the other. I do however expect to be able to tell if what I expect is consistent with what I emit. And certainly not over paranoid, but having said that -- this is a Microsoft project.
I am merely security conscious, having worked with hundreds of poorly written websites and having to had fixed hundreds of hacks. Ensuring consistency goes a long way to a secure site and reduced system overhead should an exploit otherwise occur.
@JasonCaldwell - I suggest you reread what I wrote, since you seem to be arguing against something you perceive I said, not what I did. My point is that the things you view as a security flaw in Java are generally not in ASP.NET because they are designed to mitigate those situations and they have mechanisms to deal with them, but you want to bypass all that and do something java-like which is the wrong thing to do. Specifically, read my last sentence clearly.
I'm arguing against your overall point (or points). Furthermore, I never made claim that in Java it's a flaw, if you asked I would say quite the contrary, the flexibility Java affords in this area and several others is quite excellent. In Java a lot ISN'T done for you, which means you know what its doing because you coded it. While you claim that ASP.NET is designed to mitigate "those" situations, you offer no explanation as to how. I encourage you to re-read my original post and also the link enclosed within. Testing a NULL for a NULL isn't exclusive to Java, so get over the Java kick.
|
0

@ignatandrei from ASP.NET answered the question:

by default MVC transforms empty string to null string.

http://forums.asp.net/t/2006146.aspx?Custom+Validation+and+NULL

Improve this answer

18 Comments

I don't think that affects Request.Form... was there a reason you didn't like that solution? Like, you could look at AllKeys and see fi the key isn't there.
Please explain what you mean? I don't think you understand my original question or this answer.
if (!HttpContext.Current.Request.Form.AllKeys.Contains("prop")){throw new Exception();}
Basically you're bypassing MVC and model binding altogether and just looking at the form.
I was trying to hint at that before but I edited my answer to be clearer. Anyway, hope that helps you.
|

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.