Multiple (2x) encoding detected in from PercentCodec

[kwwall]

[This question was originally posted as GitHub Issue #692 by @saravanansubiramaniam. That issue is being closed and I am manually moving the discussion to here. (GitHub is supposed to have a button for that, but it is not currently working.)

Our REST API receives the query parameters entered by the user. If the user wants to search for employees whose name ends with 2567, the query parameter looks like this: http://host:port/api?q=code.like('%2567') - so we use % for 'like' searches. However this causes error Multiple (2x) encoding detected in code.like('%2567')

Is this expected? If so, how should this search be supported?

Eg:
ESAPI.encoder().canonicalize("%2567");

[kwwall]

@xeno6696 commented:

Is this expected?

Yes.

If so, how should this search be supported?

This is much harder without understanding your application.
Your problem is that % in a URL has to be encoded to %25 because of syntax rules. And that '%' character also means something special in other languages like SQL.

I can't interpret your URL value. Please be much more explicit with your code examples. Your URL example and your encoder call don't make much sense because in a normal java application I would expect something that looks like this:

String query = request.getQuery("q"); // query.equals( "code.like('%2567')" )

So your example when thinking about traditional Java code is confusing. What's the exact URL? What's the value in debug JUST BEFORE it gets passed to ESAPI, and is it precisely a manual canonicalize call or are you passing it into a Validator method?

[kwwall]

@planetlevel replied:
Add the % on the server side…. post-canonicalization.

—Jeff

[kwwall]

@xeno6696 replied:

^^^That's more or less where I'm headed, but I wanted to understand a little bit more.

[saravanansubiramaniam]

@planetlevel replied: Add the % on the server side…. post-canonicalization.

—Jeff

@planetlevel Could you please give an example of doing this server side post-canonicalization?

[saravanansubiramaniam]

So your example when thinking about traditional Java code is confusing. What's the exact URL? What's the value in debug JUST BEFORE it gets passed to ESAPI, and is it precisely a manual canonicalize call or are you passing it into a Validator method?

The value just before it gets passed to ESAPI is "%2567". Yes, we use % as search character (in Oracle SQL).
It is precisely a manual canonicalize call: ESAPI.encoder().canonicalize("%2567");

[xeno6696]

@planetlevel replied: Add the % on the server side…. post-canonicalization.
—Jeff

@planetlevel Could you please give an example of doing this server side post-canonicalization?

There are several examples here: https://cheatsheetseries.owasp.org/cheatsheets/SQL_Injection_Prevention_Cheat_Sheet.html

the idea is that you need to remove the percent symbol from user control.

This means you could have two prepared statements in your code, one for “like” and one without. Not an elegant answer but it gets the point across.

String query = "SELECT account_balance FROM user_data WHERE user_name = ?" in non-like cases and

String query2= "SELECT account_balance FROM user_data WHERE user_name = %?"

this is to stress that you don’t want the user being able to insert the ‘%’ character themselves.

[kwwall]

@saravanansubiramaniam - If you are in fact, calling one of ESAPI's Encoder.canonicalize() methods here, I think there are 2 approaches that would work. However, if instead, Encoder.canonicalize() is being invoked via one of the Validator.getValidSafeHTML() methods, I think only one work around is possible.

The workaround that should be possible regardless of which approach you are taking is to change your application code. You stated that:

the query parameter looks like this: http://host:port/api?q=code.like('%2567') - so we use % for 'like' searches

It seems to me that the 'code.like(2567)' could be without the '%' (and you would just have to add it back in to your SQL statement after validating the input). If that's not sufficient though, you could also use some other character to match and just replace that character with a '%' after you have validated it. For example '*' is often associated with wildcard searches, so you could use 'q=code.like(*2567)'' to match an employee number ending with '2567' and 'q=code.like(2567*)' to match an employee number starting with '2567'. That obviously is a (hopefully minor) design change, and while I have not tested it, I think that approach out to work, although you may have to select something other than '*' for the SQL 'like' character.

The second choice will only work if you calling one of the Encoder.canonicalize() method directly. Instead of writing something like:

    String clean = ESAPI.encoder().canonicalize( input );

you would write something like this:

     ArrayList list = new ArrayList();
     list.add( new HTMLEntityCodec() );
     // list.add( new PercentCodec() );   // Leave this one out.
     list.add( new JavaScriptCodec() );
     Encoder encoder = new DefaultEncoder( list );
     String clean = encoder.canonicalize( request.getParameter( "input" ));

In theory, you could make something like this work but be-warned, you should only use it in the special cases where you are expecting that '%' in the query parameter to mean to translate it to the SQL 'like'. This has the downside of you are now directly using implementation classes rather than just interfaces and that has sort of a bad code smell to it, but it's in part for things like this that that particular CTOR for DefaultEncoder was made public.

[saravanansubiramaniam]

Are there any downside/security risk of skipping PercentCodec if the query parameter contains % (to use in SQL search)?

[xeno6696]

Yes, it will expose your entire application to attacks based on URL encoding.

You have to have a REALLY good reason to do this, and from what you’ve stated so far, you’re not there yet.

[kwwall]

To @xeno6696's point, that's why I stated that if you were going to use that second approach I mentioned, you would ONLY want use it in that specific case to at least limit the potential impact.

[amssaravanan]

It seems there are more issues. For example, the following JSON payload in the POST request body throws Multiple (2x) encoding detected in Some display%2542name error
{ "code": "SOME_CODE", "descr": "Some description", "displayName": "Some display%2542name" }

So, if the user wants set display name that contains %25 followed by 2 digits (as-is i.e., without encoding), it is not possible.

We also have a REST API that supports setting credentials using a POST request. Example payload:
{ "username": "secretname", "password": "MyPassword%2542" }

Same issue here. What if the user wants to set the password as literal MyPassword%2542?

In all these cases, the reported error is a false positive.

Do you have any suggestions?

[kwwall]

Again, I think we need to specifically understand the ESAPI calls that you are using where you see these calls. My gut is telling me you are trying to use the same approach for ALL parameters values regardless of what they are, but we can't tell you are using Encoder.canonicalize() or one of the Validator.getValidSafeHTML() methods or some other Validator method. Maybe your password example is a red herring, but as an example, you pretty much SHOULD allow any character or character combination for a password, the only "validation" that makes any sense at all is if users are (re)setting their passwords...then you might want to run something against that candidate password to check it against your password policy (e.g., has a certain minimum length, adheres to you companies password complexity policy, etc.) But if that passes, you store it as a salted hash as per your company policies and then when a user attempts to authenticate with their password, you recompute the hash and compare it against the stored one. If you are looking for "evil" or otherwise unsupported characters in a password, then you're doing it incorrectly. Something like "7xnh<scriPt>alert(1)</SCriPT>" should be a perfectly legal. You shouldn't care about illegal characters in a password because you should NEVER be rendering those or logging them. By comparison, if I expect the use to enter a US state abbreviation, your validation should be a comparison against the 50 possible US state abbreviations. If it's suppose to be a 5-digit US zip code, then you want to compare it 5 digits with a regex or however you choose to do so. But unless you tell us specifically what you are doing--that is, how you are coding it now--all we can do is speculate and that's likely to take several iterations to us to understand, if indeed we ever get there at all.

[xeno6696]

@amssaravanan

So, if the user wants set display name that contains %25 followed by 2 digits (as-is i.e., without encoding), it is not possible.

With your logic written as-is, this is correct.

We also have a REST API that supports setting credentials using a POST request. Example payload:
{ "username": "secretname", "password": "MyPassword%2542" }

Same issue here. What if the user wants to set the password as literal MyPassword%2542?

In all these cases, the reported error is a false positive.

Well, actually not. The design intent of the detection logic is to fail-fast: We assume all input is malicious, (as should you) and in any event where you have mixed contexts--in your case a JSON context that contains data that can also be interpreted as percent-encoded data, then we do not have a false positive--we have a positive.

But, with what you have given us, I've found the problem: You're attempting to validate the ENTIRE JSON payload.

How do I know you're doing this? Because that's the only way { "username": "secretname", "password": "MyPassword%2542" } could trigger a multiple-encoding exception. If you handle it properly, this should never happen. How do I handle it properly?

Your JSON example should be marshalled into a plain old java object (POJO). There are many convenience libraries to do this.

But you don't want to perform any input validations until you have stripped away those communication layers.

{ "username": "secretname", "password": "MyPassword%2542" }

Should be something like:

JSONObject creds = JSON.parse( input );
//where you can then snag creds.getUserName(); and creds.getPassword(); and then perform your canonicalization/input validation there.
//then later you can 

String canonicalized = encoder.canonicalize(creds.getPassword());
//And it should work fine in almost any circumstance unless it's a URL, which should be validated after
encoder.getCanonicalizedURI( input);

There can be other caveats depending on what you're doing, but those are better explained when actual code artifacts manifest actual code problems. This results from your example, the application receives the password data percent-encoded like MyPassword%2542

When designing an application from the beginning, I stipulate a rule that says no processing can happen against encoded inputs: Since the input MyPassword%2542 clearly underwent a percent-encoding pass, you should unescape it before doing any work against it. Then, you make a decision like "We will discard what we canonicalize and validate, and if valid, store the original input as-is." That's because canonicalization is a destructive operation in your case (reducing %25 to %) and you can (and probably will) introduce odd bugs downstream because those decisions--I almost guarantee it--haven't been made, and every programmer that runs into this problem in other parts of the application will handle it differently.

As both Kevin and Jeff has pointed out however, validating a password is something you will handle specially. I won't repeat them on that.

[planetlevel]

{ "code": "SOME_CODE", "descr": "Some description", "displayName": "Some display%2542name" }

When you call validate on “displayName,” ESAPI should throw the Multiple Encoding exception.” It’s dangerous to pass around strings like this because downstream systems will probably decode them. In case it’s not obvious, the %25 decodes to the percent (%) character, leaving %42. %42 in turn decodes to capital-B. So the displayName could easily end up being “displayBname” causing no end of havoc.

{ "username": "secretname", "password": "MyPassword%2542" }

In this case, you shouldn’t be calling validate on “password.” It’s a password and you don’t validate it this way. You use a good hash algorithm and salt and look that up in a database. So there’s no mistake here either. As Kevin noted, you don’t validate the entire JSON payload here at once. Good luck!

…

--Jeff From: Kevin W. Wall ***@***.***> Date: Monday, May 30, 2022 at 11:00 PM To: ESAPI/esapi-java-legacy ***@***.***> Cc: Jeff Williams ***@***.***>, Mention ***@***.***> Subject: Re: [ESAPI/esapi-java-legacy] Multiple (2x) encoding detected in from PercentCodec (Discussion #694) Again, I think we need to specifically understand the ESAPI calls that you are using where you see these calls. My gut is telling me you are trying to use the same approach for ALL parameters values regardless of what they are, but we can't tell you are using Encoder.canonicalize() or one of the Validator.getValidSafeHTML() methods or some other Validator method. Maybe your password example is a red herring, but as an example, you pretty much SHOULD allow any character or character combination for a password, the only "validation" that makes any sense at all is if users are (re)setting their passwords...then you might want to run something against that candidate password to check it against your password policy (e.g., has a certain minimum length, adheres to you companies password complexity policy, etc.) But if that passes, you store it as a salted hash as per your company policies and then when a user attempts to authenticate with their password, you recompute the hash and compare it against the stored one. If you are looking for "evil" or otherwise unsupported characters in a password, then you're doing it incorrectly. Something like "7xnh<scriPt>alert(1)</SCriPT>" should be a perfectly legal. You shouldn't care about illegal characters in a password because you should NEVER be rendering those or logging them. By comparison, if I expect the use to enter a US state abbreviation, your validation should be a comparison against the 50 possible US state abbreviations. If it's suppose to be a 5-digit US zip code, then you want to compare it 5 digits with a regex or however you choose to do so. But unless you tell us specifically what you are doing--that is, how you are coding it now--all we can do is speculate and that's likely to take several iterations to us to understand, if indeed we ever get there at all. — Reply to this email directly, view it on GitHub<#694 (reply in thread)>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/AAUUFTGHBARWML7DXWKBTVTVMV6ETANCNFSM5VTM653Q>. You are receiving this because you were mentioned.Message ID: ***@***.***>

[xeno6696]

I left the password validation on the table. My bad! (O_o)