Skip to content

Commit

Permalink
PR to fix #824 and reference to #823 (#828)
Browse files Browse the repository at this point in the history
* Updated DefaultEncoder.getCanonicalizedURI(URI) javadoc to indicate that the method takes into consideration canonicalization of mixed/multi encoded URLs as specified in ESAPI.props 'allowMixed' and 'allowMultiple' accordingly.

* Per issue #824.  Updated DefaultEncoder.getCanonicalizedURI(URI) javadoc to indicate that the method takes into consideration canonicalization of mixed/multi encoded URLs as specified in ESAPI.props 'allowMixed' and 'allowMultiple' accordingly.

* Fixed #824 by nesting the original canonicalize call into the else block of the check to see whether or not we were dealing with a query segment.
  • Loading branch information
xeno6696 authored May 27, 2024
1 parent 7a9ec00 commit f45876f
Show file tree
Hide file tree
Showing 4 changed files with 122 additions and 20 deletions.
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/**
* OWASP Enterprise Security API (ESAPI)
* OWASP Enterprise Security API (ESAPI)
*
* This file is part of the Open Web Application Security Project (OWASP)
* Enterprise Security API (ESAPI) project. For details, please see
Expand Down
25 changes: 16 additions & 9 deletions src/main/java/org/owasp/esapi/reference/DefaultEncoder.java
Original file line number Diff line number Diff line change
Expand Up @@ -520,6 +520,9 @@ public byte[] decodeFromBase64(String input) throws IOException {
* This will extract each piece of a URI according to parse zone as specified in <a href="https://www.ietf.org/rfc/rfc3986.txt">RFC-3986</a> section 3,
* and it will construct a canonicalized String representing a version of the URI that is safe to
* run regex against.
*
* NOTE: This method will obey the ESAPI.properties configurations for allowing
* Mixed and Multiple Encoding URLs.
*
* @param dirtyUri
* @return Canonicalized URI string.
Expand Down Expand Up @@ -548,7 +551,6 @@ public String getCanonicalizedURI(URI dirtyUri) throws IntrusionException{
parseMap.put(UriSegment.SCHEME, dirtyUri.getScheme());
//authority = [ userinfo "@" ] host [ ":" port ]
parseMap.put(UriSegment.AUTHORITY, dirtyUri.getRawAuthority());
parseMap.put(UriSegment.SCHEMSPECIFICPART, dirtyUri.getRawSchemeSpecificPart());
parseMap.put(UriSegment.HOST, dirtyUri.getHost());
//if port is undefined, it will return -1
Integer port = new Integer(dirtyUri.getPort());
Expand All @@ -557,9 +559,6 @@ public String getCanonicalizedURI(URI dirtyUri) throws IntrusionException{
parseMap.put(UriSegment.QUERY, dirtyUri.getRawQuery());
parseMap.put(UriSegment.FRAGMENT, dirtyUri.getRawFragment());

//Now we canonicalize each part and build our string.
StringBuilder sb = new StringBuilder();

//Replace all the items in the map with canonicalized versions.

Set<UriSegment> set = parseMap.keySet();
Expand All @@ -568,8 +567,7 @@ public String getCanonicalizedURI(URI dirtyUri) throws IntrusionException{
boolean allowMixed = sg.getBooleanProp("Encoder.AllowMixedEncoding");
boolean allowMultiple = sg.getBooleanProp("Encoder.AllowMultipleEncoding");
for(UriSegment seg: set){
String value = canonicalize(parseMap.get(seg), allowMultiple, allowMixed);
value = value == null ? "" : value;
String value = "";
//In the case of a uri query, we need to break up and canonicalize the internal parts of the query.
if(seg == UriSegment.QUERY && null != parseMap.get(seg)){
StringBuilder qBuilder = new StringBuilder();
Expand Down Expand Up @@ -597,6 +595,10 @@ public String getCanonicalizedURI(URI dirtyUri) throws IntrusionException{
} catch (UnsupportedEncodingException e) {
logger.debug(Logger.EVENT_FAILURE, "decoding error when parsing [" + dirtyUri.toString() + "]");
}
} else {
String extractedInput = parseMap.get(seg);
value = canonicalize(extractedInput, allowMultiple, allowMixed);
value = value == null ? "" : value;
}
//Check if the port is -1, if it is, omit it from the output.
if(seg == UriSegment.PORT){
Expand All @@ -618,11 +620,16 @@ public String getCanonicalizedURI(URI dirtyUri) throws IntrusionException{
*/
protected String buildUrl(Map<UriSegment, String> parseMap){
StringBuilder sb = new StringBuilder();
sb.append(parseMap.get(UriSegment.SCHEME))
.append("://")
boolean schemePresent = parseMap.get(UriSegment.SCHEME).equals("") ? false : true;

if(schemePresent) {
sb.append(parseMap.get(UriSegment.SCHEME))
.append("://");
}

//can't use SCHEMESPECIFICPART for this, because we need to canonicalize all the parts of the query.
//USERINFO is also deprecated. So we technically have more than we need.
.append(parseMap.get(UriSegment.AUTHORITY) == null || parseMap.get(UriSegment.AUTHORITY).equals("") ? "" : parseMap.get(UriSegment.AUTHORITY))
sb.append(parseMap.get(UriSegment.AUTHORITY) == null || parseMap.get(UriSegment.AUTHORITY).equals("") ? "" : parseMap.get(UriSegment.AUTHORITY))
.append(parseMap.get(UriSegment.PATH) == null || parseMap.get(UriSegment.PATH).equals("") ? "" : parseMap.get(UriSegment.PATH))
.append(parseMap.get(UriSegment.QUERY) == null || parseMap.get(UriSegment.QUERY).equals("")
? "" : "?" + parseMap.get(UriSegment.QUERY))
Expand Down
20 changes: 20 additions & 0 deletions src/test/java/org/owasp/esapi/codecs/HTMLEntityCodecTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

import static org.junit.Assert.assertEquals;

import org.junit.Ignore;
import org.junit.Test;

public class HTMLEntityCodecTest {
Expand Down Expand Up @@ -48,4 +49,23 @@ public void testMixedBmpAndNonBmp(){
String input = bmp + nonBMP;
assertEquals(expected, codec.encode(new char[0], input));
}

@Test
/**
* TODO: The following methods are unit tests I'm checking in for an issue to be worked and fixed.
*/
@Ignore("Pre check-in for issue #827")
public void testIssue827() {
String input = "/webapp/ux/home?d=1705914006565&status=login&ticket=1705914090394_HzJpTROVfhW-JhRW0OqDbHu7tWXXlgrKSUmOzIMsZNCcUIiYGMXX_Q%3D%3D&newsess=false&roleid=DP010101/0007&origin=ourprogram";
String expected = input;
assertEquals(expected, codec.decode(input));
}

@Test
@Ignore("Pre check-in for issue #827")
public void testIssue827OnlyOR() {
String input = "&origin=ourprogram";
String expected = input;
assertEquals(expected, codec.decode(input));
}
}
95 changes: 85 additions & 10 deletions src/test/java/org/owasp/esapi/reference/EncoderTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -15,25 +15,21 @@
*/
package org.owasp.esapi.reference;

import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertNotEquals;

import java.io.IOException;
import java.io.UnsupportedEncodingException;
import java.net.URI;
import java.util.List;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.HashMap;
import java.util.Map;
import java.util.Map.Entry;
import java.util.regex.Matcher;
import java.util.regex.Pattern;
import java.util.List;

import org.junit.Ignore;
import org.owasp.esapi.ESAPI;
import org.owasp.esapi.Encoder;
import org.owasp.esapi.EncoderConstants;
import org.owasp.esapi.codecs.CSSCodec;
import org.owasp.esapi.SecurityConfiguration;
import org.owasp.esapi.SecurityConfigurationWrapper;
import org.owasp.esapi.codecs.Codec;
import org.owasp.esapi.codecs.HTMLEntityCodec;
import org.owasp.esapi.codecs.MySQLCodec;
Expand All @@ -45,8 +41,7 @@
import org.owasp.esapi.errors.EncodingException;
import org.owasp.esapi.errors.IntrusionException;
import org.owasp.esapi.Randomizer;
import org.owasp.esapi.SecurityConfiguration;
import org.owasp.esapi.SecurityConfigurationWrapper;


import junit.framework.Test;
import junit.framework.TestCase;
Expand Down Expand Up @@ -747,6 +742,7 @@ public void testDecodeFromURL() throws Exception {
fail();
}
try {
//FIXME: Rewrite this to use expected Exceptions.
instance.decodeFromURL( "%3xridiculous" );
fail();
} catch( Exception e ) {
Expand Down Expand Up @@ -985,6 +981,50 @@ public void testGetCanonicalizedUri() throws Exception {
assertEquals(expectedUri, e.getCanonicalizedURI(uri));

}

public void testGetCanonicalizedUriWithAnHTMLEntityCollision() throws Exception {
System.out.println("GetCanonicalizedUriWithAnHTMLEntityCollision");
Encoder e = ESAPI.encoder();

String expectedUri = "http://[email protected]/path_to/resource?foo=bar&para1=test";
//Please note that section 3.2.1 of RFC-3986 explicitly states not to encode
//password information as in http://palpatine:[email protected], and this will
//not appear in the userinfo field.
String input = "http://[email protected]/path_to/resource?foo=bar&para1=test";
URI uri = new URI(input);
System.out.println(uri.toString());
assertEquals(expectedUri, e.getCanonicalizedURI(uri));

}

@org.junit.Ignore("Pre-check in unit test for issue #826")
public void Issue826GetCanonicalizedUriWithMultipleEncoding() throws Exception {
System.out.println("GetCanonicalizedUriWithAnHTMLEntityCollision");
Encoder e = ESAPI.encoder();
String expectedUri = "http://[email protected]/path_to/resource?foo=bar&para1=&amp;amp;amp;test";
//Please note that section 3.2.1 of RFC-3986 explicitly states not to encode
//password information as in http://palpatine:[email protected], and this will
//not appear in the userinfo field.
String input = "http://[email protected]/path_to/resource?foo=bar&para1=&amp;amp;amp;test";
URI uri = new URI(input);
System.out.println(uri.toString());
assertEquals(expectedUri, e.getCanonicalizedURI(uri));

}
public void testGetCanonicalizedUriWithMultQueryParams() throws Exception {
System.out.println("getCanonicalizedUri");
Encoder e = ESAPI.encoder();

String expectedUri = "http://palpatine@foo bar.com/path_to/resource?foo=bar&bar=foo#frag";
//Please note that section 3.2.1 of RFC-3986 explicitly states not to encode
//password information as in http://palpatine:[email protected], and this will
//not appear in the userinfo field.
String input = "http://palpatine@foo%20bar.com/path_to/resource?foo=bar&bar=foo#frag";
URI uri = new URI(input);
System.out.println(uri.toString());
assertEquals(expectedUri, e.getCanonicalizedURI(uri));

}

public void testGetCanonicalizedUriPiazza() throws Exception {
System.out.println("getCanonicalizedUriPiazza");
Expand All @@ -1000,6 +1040,41 @@ public void testGetCanonicalizedUriPiazza() throws Exception {
assertEquals(expectedUri, e.getCanonicalizedURI(uri));

}

public void testIssue824() throws Exception {
System.out.println("getCanonicalizedUriPiazza");
Encoder e = ESAPI.encoder();

String expectedUri = "/webapp/ux/home?d=1705914006565&status=login&ticket=1705914090394_HzJpTROVfhW-JhRW0OqDbHu7tWXXlgrKSUmOzIMsZNCcUIiYGMXX_Q==&newsess=false&roleid=DP010101/0007&origin=ourprogram";
//Please note that section 3.2.1 of RFC-3986 explicitly states not to encode
//password information as in http://palpatine:[email protected], and this will
//not appear in the userinfo field.
String input = "/webapp/ux/home?d=1705914006565&status=login&ticket=1705914090394_HzJpTROVfhW-JhRW0OqDbHu7tWXXlgrKSUmOzIMsZNCcUIiYGMXX_Q%3D%3D&newsess=false&roleid=DP010101/0007&origin=ourprogram";
URI uri = new URI(input);
System.out.println(uri.toString());
assertEquals(expectedUri, e.getCanonicalizedURI(uri));

}

@org.junit.Ignore("Pre-check in unit test for issue #826")
public void Issue826GetCanonicalizedDoubleAmpersand() throws Exception {
System.out.println("getCanonicalizedDoubleAmpersand");
Encoder e = ESAPI.encoder();
String expectedUri = "http://127.0.0.1:3000/campaigns?goal=all&section=active&sort-by=-id&status=Draft%2C&html=&amp;contentLaunched";
//http://127.0.0.1:3000/campaigns?goal=all&section=active&sort-by=-id&status=Draft,&html=null&=null&amp;contentLaunched=null
/*
* In this case, the URI class should break up the HTML entity in the query so
*/
String input = "http://127.0.0.1:3000/campaigns?goal=all&section=active&sort-by=-id&status=Draft%2C&html=&&amp;contentLaunched";
URI uri = new URI(input);
System.out.println(uri.toString());
try {
assertEquals(expectedUri, e.getCanonicalizedURI(uri));
fail();
} catch (Exception ex) {
//Expected
}
}

public void testGetCanonicalizedUriWithMailto() throws Exception {
System.out.println("getCanonicalizedUriWithMailto");
Expand Down

0 comments on commit f45876f

Please sign in to comment.