Http -Parameter Value Length / Validation issue

[RobertMolenda]

Describe the bug

Currently utilizing release 2.5.5.0 - looked at 2.6.0.0 and code is the same - so here it comes:

There are two observed issues with http parameter validation regarding "Length" checks

Observation 1: function:
SecurityWrapperRequest.public String[] getParameterValues(String name) {...}

Utilizes the setting:
HttpUtilities.URILENGTH
to validate the length of the value
Recommend adding a new parameter to differentiate "this vs that" so to say

Observation 2: Setting
The default setting for regex on httpParameterValue is:

Validator.HTTPParameterValue=^[-\p{L}\p{N}./+=_ !$*?@]{0,1000}$

This setting limits the value to 1000 bytes

To Reproduce

Create an http parameter > URILENGTH and send through cleansing

Expected behavior

Provide different length-settings between httpParameterValues versus URILENGTH
or
Remove the length check - and allow the regex to control validation

Reasoning:
they need to be validated differently

URL Length - prevent spamming an application
HttpParameter Length - prevent hacking / crashing an application

note granted - changing the MAXURI setting as well as reconfiguration of the HTTPParameterValue Regex statement is my current work-around, however the IT Security Team isn't that happy having a maximum URL Length that is massive.

[kwwall]

Okay, maybe I'm just having an off day or am extra tired, or maybe @RobertMolenda just wasn't clear what the 'bug' is here and what we are being asked to do correct the situation.

However, looking at the SecurityWrapperRequest.java file, line 455, I do see an oddity in the getParameterValues method:

String cleanValue = ESAPI.validator().getValidInput("HTTP parameter value: " + value, value, "HTTPParameterValue", sc.getIntProp("HttpUtilities.URILENGTH"), true);

If the regex for value fails against the regex check as given by the Validator.HTTPParameterValue property, if will fail in one way, but if the total length for value exceeds HttpUtilities.URILENGTH, it will fail differently. I guess one could argue that that inconsistency is wrong, but I don't see how it is insecure. Just perhaps a bit confusing.

The problem is that we don't have a separate property for the maximum length of a separate parameter value. We could fix this by adding one and passing that in for the maxlength. So this issue is we're incorrectly comparing the overall URI length with the length of a specific single named parameter value. And if that parameter came from a POST rather than a GET, using HttpUtilities.URILENGTH makes zero sense because POST parameters are not really part of the URI and thus not included in the URI length.

The problem with fixing this and changing it to do something that is more intuitive means potentially (I could say likely) break backward compatibility. And since both of these 2 properties

• HttpUtilities.URILENGTH
• Validator.HTTPParameterValue

and thus can be tweaked as needed, I am somewhat reluctant to change the Validator.HTTPParameterValue regex to match the HttpUtilities.URLLENGTH as the max length of an HTTP parameter value. Alternately, in theory at least, one ought to be able to extend SecurityWrapperRequest and just override that one getParameterValues method.

I guess at this point, I need to here more convincing evidence about why this should be changed. I personally view more as confusing than wrong. And it certainly is not insecure in my mind. (I might consider it wrong if HttpUtilities.URLLENGTH were set to 1000 or less.)

In closing, I'm a bit puzzled about @RobertMolenda's comment of:

note granted - changing the MAXURI setting as well as reconfiguration of the HTTPParameterValue Regex statement is my current work-around, however the IT Security Team isn't that happy having a maximum URL Length that is massive.

I mean, what in the world did you change these to parameters too that your IT Security Team isn't happy with a "massive" URL length. If you look at the StackOverflow reference in the comment just above HttpUtilities.URLLENGTH, it notes that most browsers accept 8000 octets as per the RFC. Plus there are other (and IMHO, better) ways to enforce a maximum URL length. I now that Tomcat allows you to configure that and I expect that other JavaEE / Jakarta servlet engines / application servers allow that as well. So, I'm really not getting their alarm at all.

Anyway, please give me some better rationale as to why this should be fixed. Thus far, unless @xeno6696 or @jeremiahjstacey convince me otherwise, I am leaning towards closing this and marking it as "wontfix".

-kevin
P.S.- It's days like this when I think that ESAPI should have been named TMDP (Too Many Damned Properties). Sigh.

[xeno6696]

I completely agree that there's some uncovered tech debt here..

I have a hard time determining what the security issue could be with default settings. Obviously there's four cases to consider because of there being two settings, but agreed, all I see here are unexpected instances of whichever value is shorter causing a rejection which would cause someone to debug and determine that the value is set in two places after some hunting.

And then yes, I can see there being some head-scratching as it involves the idea that there's a context switch between a GET parameter value and a POST parameter value.

I don't necessarily think that backwards compatibility should be honored if this behavior causes a security issue, as the only way I can see this being a problem is if an ESAPI user alters the defaults. By default the regex has a length and the method call itself requires a length even if you're passing in your own value.

The scarier part is that there aren't as many unit tests for this part of the code, so the chances of unforseen breaks for clients would definitely be something to consider.

If we do decide to do something here, I'll volunteer, but I don't believe this is urgent.

[RobertMolenda]

Well - honestly - "Bug" isn't a great indication - as well the code doesn't explode. - nor - is there a potential security issue

With that being said - as xeno6696 pointed out - this is more 'technical debt' that could be addressed in R3 which you guys are working on.

My dog-chasing-squirrel-thinking about this is that the URILENGTH should be utilized to check URL Lengths and not "other things that are not URL related" - and that the HTTPParameter-Length/Value Checks have currently two methods of checking

The "length" setting and/or the length presented in Regex.

That and/or* allows for a configuration to blow hole in foot (been there done that) - in where

Configure length for X and Regex validates for length Y and depending on that - then your foot hurts or not.

So in a perfect "configuration safe world" - the Regex expression would be checked for a 'length configuration' and if so then you can (squirrel mode) - validate against a "length configuration" OR use one or the other... (trust me in saying I write a bunch of validator-rules to ensure the configurator-person doesn't hurt themselves)

Certainly the architecture of the application I inherited "has room for improvement" - and a 15K header-parameter-value isn't optimal in my mind either - but changing architecture is a good few (many ??) releases out...

So to summarize - in looking at "Configuration Variable Names" - one makes assumptions (bad) - in where URILength does just that - and not be utilized for 'other checks' not related to URI

Best
Robert