[OWASP-ESAPI] Validator class - inadequate validation
Jim Manico
jim at manico.net
Thu Jan 24 02:21:19 EST 2008
> The details of data length is a difficult one to deal with. Could
this possibly become part of the ESAPI properties file?
I do not think there is a right or wrong answer, but we need to be aware
of the trade offs.
If you push the data length validation (as well as the regEx) outside of
the application into the configuration layer, it's makes it easy for a
non-programmer to configure and secure the application's input
validation "after the fact" of coding. You also get to find tune the
security of the application without programmer intervention.
But decoupling input validation from the application itself has risks
that we should be aware of. It's easy to lose site of even one field. It
will be more difficult to audit the code (debatable), the code itself
will not be secure without the configuration layer.
Since programmers are so bad, traditionally, at securing apps, I'm
begrudgingly starting to agree with the wisdom of choice A (and EASPI's
choice). I've had several *very large* clients recently *require* in
the original bid that each and every form field map to a regular
expression the configuration layer. I give up. :)
- Jim
> This conversation is a good one to get the brain warmed up in the
> morning! Some excellent points put across here.
>
> /*public* String getValidDataFromBrowser(String context, String type,
> String input)
> /
> Yes! I didn't see that and I like this alot.
>
> > If you can come up with a API level/RegEx hybrid, I think it will
> add to the power of >the API.
>
> Very valid point and I do agree it will improve the richness of the
> ESAPI API. I think it must be approached with caution, due to the
> power and fliexbility of regex we could obviously write methods all
> day which would take arguments for a regex, some of which would become
> overly complex. For example, do we start putting in methods which deal
> with the formatting of data too? (I certainly wouldn't recommend it).
> I do like the idea of this API/regex hybrid approach provided the
> exposed functions remain simple, as I said it can easily get carried
> away.
>
> The details of data length is a difficult one to deal with. Could this
> possibly become part of the ESAPI properties file?
>
>
>
>
> On Jan 22, 2008 8:56 PM, Jim Manico < jim at manico.net
> <mailto:jim at manico.net>> wrote:
>
> The "context" addition for 1.1 is very useful, nice addition. I'll
> surrender to regular expressions at the configuration level and
> will give up on my Alpha/Numeric helper functions.
>
> > The length option is more difficult. I can see that many fields
> will have the same regex, except for the length. So as much as I
> want one-and-only-one place to define validation rules, I can see
> the benefit of saying a field is five numeric characters. I need
> to think on this some more.
>
> This is exactly the issue I was trying to address. If you can come
> up with a API level/RegEx hybrid, I think it will add to the power
> of the API. This is also very critical for Alpha or hybrid fields
> as well. Even email type regular expressions should be protected
> in such a way, IMO so a overly long but legal email address with
> the intention of trying to force a stack trace fails gracefully.
>
> Right on, great discussion Adam/Jeff,
> Jim
>
>> Certainly for the numbers I think this is a great idea. I'm
>> pretty surprised that I didn't do it that way. I'm not sure
>> about the Alpha convenience methods you suggest. Note that the
>> API changed a bit in version 1.1….
>>
>>
>>
>> *public* String getValidDataFromBrowser(String context,
>> String type, String input)
>>
>>
>>
>> The "context" is a string that indicates where the data came from
>> – like the name of the request parameter. This makes the log
>> entry very explicit. The "type" is the **name** of the regex in
>> the ESAPI.properties file – like "AlphaNumeric". This makes it
>> very easy to define validators and access them from anywhere.
>>
>>
>>
>> So for your example below you would say:
>>
>>
>>
>> getValidDataFromBrowser("EmpName", "EmployeeName",
>> request.getParameter( "EmpName" ));
>>
>>
>>
>> I'm not crazy about the extraAllowedCharacters option. I want to
>> encourage people to specifically define the type of data they're
>> accepting into their application.
>>
>>
>>
>> The length option is more difficult. I can see that many fields
>> will have the same regex, except for the length. So as much as I
>> want one-and-only-one place to define validation rules, I can see
>> the benefit of saying a field is five numeric characters. I need
>> to think on this some more.
>>
>>
>>
>> You can see a simplification in the current signature if we use
>> the current request threadlocal - it could automatically get the
>> parameter from the request for you. But that would make ESAPI
>> more like a framework, which I'd like to avoid. I think having a
>> set of good primitive security methods is the goal. Frameworks
>> can layer simplifications on top if they like.
>>
>>
>>
>> I'm glad you guys are helping out with the Validator API. I
>> spent much more of the design time on some of the more innovative
>> APIs in ESAPI.
>>
>>
>>
>> --Jeff
>>
>>
>>
>> *From:* Jim Manico [mailto:jim at manico.net]
>>
>> *Sent:* Tuesday, January 22, 2008 2:24 PM
>> *To:* jeff.williams at owasp.org <mailto:jeff.williams at owasp.org>
>> *Cc:* 'Adam Boulton'; 'owasp-esapi'
>>
>> *Subject:* Re: [OWASP-ESAPI] Validator class - inadequate validation
>>
>>
>>
>> Jeff,
>>
>> What do you think about adding parameters for range checking?
>> Should this logic be pushed to the regular expression?
>>
>> validator.getValidDataFromBrowserAlphaNumeric(String
>> otherAllowedCharacters,* int maxNumCharacters,* boolean
>> isRequired, String value)
>> validator.getValidDataFromBrowserAlphaOnly(String
>> otherAllowedCharacters, *int maxNumCharacters, *boolean
>> isRequired, String value)
>> validator.getValidInteger(*int minValue, int maxValue,* boolean
>> isRequired, String value)
>> validator.getValidFloat(*float minValue, float maxValue,* boolean
>> isRequired, String value)
>>
>> Adam, with respect I submit that this is not making the API
>> overly complex, but is a part of a basic whitelist validation
>> strategy that I would use day-2-day in enterprise development.
>>
>> I also have no problem using regular expressions for complex
>> validation like email addresses, but to limit a String by a few
>> characters, it seems like overkill and unnecessary separation of
>> duties. And I do nod to the other school of thought (ala Struts)
>> that says that ALL form parameters should be pushed to the
>> configuration layer via RegEx : but only if you have the
>> resources to do so. :)
>>
>> - Jim
>>
>>
>> Great discussion guys – You all have got me thinking hard about
>> how to support optional parameters in a way that's easy and
>> intuitive for developers. Let's keep searching for a great API!
>>
>>
>>
>> Note that there is a method that does something related:
>>
>>
>>
>> isValidParameterSet( Set required, Set optional )
>>
>>
>>
>> The idea was that this would prevalidate a request to determine
>> that ALL required parameters are present, and the ONLY other
>> parameters are in the optional set. Any other parameters
>> provided are probably an attack.
>>
>>
>>
>> Still, it feels like there's some room to improve the Validation
>> API for optional parameters!
>>
>>
>>
>> > I would love to sit around a table and thrash all the ideas out!
>>
>>
>>
>> That would really be great. I'm so tired of "research" that's
>> focused on some obscure bug in some product. Maybe we can plan an
>> event someday if ESAPI really catches on.
>>
>>
>>
>> --Jeff
>>
>>
>>
>> *From:* owasp-esapi-bounces at lists.owasp.org
>> <mailto:owasp-esapi-bounces at lists.owasp.org>
>> [mailto:owasp-esapi-bounces at lists.owasp.org] *On Behalf Of *Adam
>> Boulton
>> *Sent:* Tuesday, January 22, 2008 6:03 AM
>> *To:* owasp-esapi
>> *Subject:* Re: [OWASP-ESAPI] Validator class - inadequate validation
>>
>>
>>
>> Hi Jim,
>>
>> I think we must be careful that the method signatures do not
>> become too complex and perform too much as it loses it's highly
>> cohesive characteristic (I am not dismissing your idea at all, I
>> find your approach valid and I can see where you are coming
>> from). In my personal opinion I see the methods as being overly
>> complex, but I appreciate we are just throwing ideas around here.
>> I appreciate what you are saying about the regex expression too
>> but I am not sure we should be trying to compensate for a
>> Developers lack of skills. If they don't know regex now would be
>> a good time for them to learn ;)
>> Just a quick question... what would address2 be assigned if the
>> boolean was false or are you expecting a ValidationException?
>>
>> /String address2 = validator.getValidDataFromBrowser(".#", 50,
>> false, request.getParameter("address2"));/
>>
>> As a new concept, to tailor for your suggestion, I am now
>> thinking we could associate a new instance of Validator to the
>> requirements of specific types of HTML forms (bare with me here,
>> i'm still piecing the idea together in my mind). So sticking to
>> your example of a US address maybe we could go along the lines
>> of.....
>>
>> /String non_required_fields = {"address2"}; //Anonymous array
>> containing fields which are allowed to be null (i suppose we
>> should whitelist instead of blacklisting :s )
>>
>> Validator usAddress = Validator.getInstance
>> (non_required_fields); //This Validator object specifically deals
>> with a us address. Construct a Validator with the non-mandatory
>> fields / null fields. address2 is not required for the US address./
>>
>> /String address1 = validator.getValidDataFromBrowser(".#",
>> 50, request.getParameter("address1"));/
>>
>> /String address2 =
>> validator.getValidDataFromBrowser(".#", 50,
>> request.getParameter("address2"));
>> String city =
>> validator.getValidDataFromBrowserAlphaOnly("", 30,
>> request.getParameter("city"));
>> String state =
>> validator.getValidDataFromBrowserAlphaOnly("", 12,
>> request.getParameter("state"));
>>
>> /Note that the boolean is no longer required, as it can be
>> determined based on the use of the overloaded factory method as
>> the names of the nullable fields were passed. The logic would
>> then need updating in getValidDataFromBrowser which would use
>> this list to determine which fields can be null and act
>> accordingly. I think this approach could tie nicely in with
>> Spring. I would love to sit around a table and thrash all the
>> ideas out!
>>
>> Cheers
>>
>> Adam
>>
>>
>> /
>>
>>
>> /
>>
>> On Jan 22, 2008 8:55 AM, Jim Manico <jim at manico.net
>> <mailto:jim at manico.net>> wrote:
>>
>> Adam,
>>
>> The current API requires that I push the required condition all
>> the way to the regular expression or add unnecessary code. Not to
>> mention that it does not do range checking (both in terms of
>> value and maximum characters) in an of itself - again I would
>> need to push that to the regular expression/configuration layer.
>>
>> Let's take a simple dispatcher controller from a standard MVC
>> J2EE architecture that is processing an HTML Form. This is the
>> core of all enterprise applications.
>>
>> So my form, is say, a US address form with the fields:
>>
>> address1 (textfield)
>> address2 (textfield)
>> city (textfield)
>> state (textfield)
>> zip (textfield)
>>
>> Address1, city state and zip are required field. Address2 is
>> optional.
>>
>> With the current Validator class, I would need to do the following.
>>
>> String address1 = validator.getValidDataFromBrowser(this,
>> "address_field", request.getParameter("address1"));
>> //example 1 of an optional field being pushed to the regEx
>> String address2 = validator.getValidDataFromBrowser(this,
>> "address_field_null_ok", request.getParameter("address1"));
>> //or
>> //example 2 of an optional field where I would need to check if
>> the result is null first
>> String address2 = request.getParameter("address1");
>> if (address2==null) throw RequiredException("address1");
>> address2 = validator.getValidDataFromBrowser(this,
>> "address_field", request.getParameter("address1"));
>>
>> String city =
>> validator.getValidDataFromBrowser(this, "city",
>> request.getParameter("city"));
>> String state =
>> validator.getValidDataFromBrowser(this, "state",
>> request.getParameter("state"));
>> String zip =
>> validator.getValidDataFromBrowser(this,
>> "zip",request.getParameter("zipCode"));
>>
>> *What I would like to recommend is to add the option to do is
>> more straightforward white list validation at the API level if
>> the form of:
>>
>> validator.getValidDataFromBrowserAlphaNumeric(String
>> otherAllowedCharacters, int maxNumCharacters, boolean isRequired,
>> String value)
>> validator.getValidDataFromBrowserAlphaOnly(String
>> otherAllowedCharacters, int maxNumCharacters, boolean isRequired,
>> String value)
>> validator.getValidInteger(int minValue, int maxValue, boolean
>> isRequired, String value)
>> validator.getValidFloat(float minValue, float maxValue, boolean
>> isRequired, String value)*
>>
>> String address1 = validator.getValidDataFromBrowser(".#",
>> 50, true, request.getParameter("address1"));
>> String address2 = validator.getValidDataFromBrowser(".#",
>> 50, false, request.getParameter("address2"));
>> String city =
>> validator.getValidDataFromBrowserAlphaOnly("", 30, true,
>> request.getParameter("city"));
>> String state =
>> validator.getValidDataFromBrowserAlphaOnly("", 12,
>> request.getParameter("state"));
>> //split zip into 2 fields that are integers only for US apps
>> String zip1 = validator.getValidInteger(1, 99999,
>> true, request.getParameter("zipCode1"));
>> String zip2 = validator.getValidInteger(1, 9999,
>> false, request.getParameter("zipCode2"));
>>
>> Most programmers that I know, even rather senior architects, do
>> not speak regular expression. That's more of an artifact of the
>> security industry. I would conjecture that if we want to make
>> ESAPI easy for programmers, then perhaps give them an option to
>> add security at the API level.
>>
>> It's also a lot less complex to create and audit.
>>
>> I'm not saying no to the current validation API, I'm only saying
>> "Yes, and..."
>>
>> - Jim
>>
>> Hi Jim, can you expand on your reasoning behind why this
>> should be an implementation on the part of the Validator API?
>> I don't believe this should be tied in with the logic in
>> Validator. If the fields which are being passed do not
>> require processing (as they are non-required elements) then
>> why are they being validated? i.e if they are non-required
>> form elements why pass them to the Validator anyway? I may be
>> missing your point here, so please elaborate for me.
>>
>> Cheers
>>
>> Adam
>>
>> On Jan 21, 2008 10:22 PM, Jim Manico < jim at manico.net
>> <mailto:jim at manico.net>> wrote:
>>
>> In web development, it's *very* common where I need to accept
>> a null from the request. (Non-required form elements).
>>
>> Please consider adding a
>>
>> "boolean required"
>>
>> To many of these API's.
>>
>> For example:
>>
>> /isValidNumber(String s, *boolean required*)/
>>
>> |/getValueDate(java.lang.String context,/|
>>
>> |/ java.lang.String input,/|
>>
>> |/ java.text.DateFormat format,/|
>>
>> |/ /|*/boolean required/*|/)/|
>>
>> etc
>>
>> - Jim
>>
>>
>>
>>
>>
>> Hi,
>>
>> The method signature:
>> /
>> boolean isValidNumber(String s)
>> /
>> which is located in the Validator class does not contain
>> adequate validation. For example, it is possible to return
>> /true/ if the value "NaN" or "Infinity" is passed as an
>> argument. If we dig down the package tree we can see the
>> source of this problem lies within FloatingDecimal object
>> which contains the character arrays for these strings and
>> will return /true /when these particular strings are passed,
>> essentially the /Double.parseDouble(String s)/ is just a
>> wrapper for /public static
>> FloatingDecimal.readJavaFormatString( String in ) throws
>> NumberFormatException /. I would argue that in this
>> particular circumstance /isValidNumber(String s) /should
>> return /false/ if "NaN" or "Infinity" is passed as an argument.
>>
>> As another note what is the purpose of having an
>> instantiation of Validator? Why are the methods, such as
>> /isValidNumber(String in)/ not static? This seems very much
>> like a helper class to me.
>>
>> Cheers
>>
>>
>>
>> ------------------------------------------------------------------------
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>> _______________________________________________
>>
>> OWASP-ESAPI mailing list
>>
>> OWASP-ESAPI at lists.owasp.org <mailto:OWASP-ESAPI at lists.owasp.org>
>>
>> https://lists.owasp.org/mailman/listinfo/owasp-esapi
>>
>>
>>
>>
>>
>>
>> _______________________________________________
>> OWASP-ESAPI mailing list
>> OWASP-ESAPI at lists.owasp.org <mailto:OWASP-ESAPI at lists.owasp.org>
>> https://lists.owasp.org/mailman/listinfo/owasp-esapi
>>
>>
>>
>>
>>
>> ------------------------------------------------------------------------
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>> _______________________________________________
>>
>> OWASP-ESAPI mailing list
>>
>> OWASP-ESAPI at lists.owasp.org <mailto:OWASP-ESAPI at lists.owasp.org>
>>
>> https://lists.owasp.org/mailman/listinfo/owasp-esapi
>>
>>
>>
>>
>>
>>
>>
>>
>> ------------------------------------------------------------------------
>>
>>
>>
>>
>> _______________________________________________
>> OWASP-ESAPI mailing list
>> OWASP-ESAPI at lists.owasp.org <mailto:OWASP-ESAPI at lists.owasp.org>
>> https://lists.owasp.org/mailman/listinfo/owasp-esapi
>>
>>
>>
>>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: https://lists.owasp.org/pipermail/owasp-esapi/attachments/20080123/1506928d/attachment-0001.html
More information about the OWASP-ESAPI
mailing list