Skip to content

Commit 3807f52

Browse files
xeno6696xeno6696
committed
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.
1 parent 7c4f232 commit 3807f52

File tree

4 files changed

+118
-20
lines changed

4 files changed

+118
-20
lines changed

‎src/main/java/org/owasp/esapi/codecs/LegacyHTMLEntityCodec.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/**
2-
* OWASP Enterprise Security API (ESAPI)
2+
* OWASP Enterprise Security API (ESAPI)
33
*
44
* This file is part of the Open Web Application Security Project (OWASP)
55
* Enterprise Security API (ESAPI) project. For details, please see

‎src/main/java/org/owasp/esapi/reference/DefaultEncoder.java‎

Lines changed: 13 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -522,7 +522,6 @@ public String getCanonicalizedURI(URI dirtyUri) throws IntrusionException{
522522
parseMap.put(UriSegment.SCHEME, dirtyUri.getScheme());
523523
//authority = [ userinfo "@" ] host [ ":" port ]
524524
parseMap.put(UriSegment.AUTHORITY, dirtyUri.getRawAuthority());
525-
parseMap.put(UriSegment.SCHEMSPECIFICPART, dirtyUri.getRawSchemeSpecificPart());
526525
parseMap.put(UriSegment.HOST, dirtyUri.getHost());
527526
//if port is undefined, it will return -1
528527
Integer port = new Integer(dirtyUri.getPort());
@@ -531,9 +530,6 @@ public String getCanonicalizedURI(URI dirtyUri) throws IntrusionException{
531530
parseMap.put(UriSegment.QUERY, dirtyUri.getRawQuery());
532531
parseMap.put(UriSegment.FRAGMENT, dirtyUri.getRawFragment());
533532

534-
//Now we canonicalize each part and build our string.
535-
StringBuilder sb = new StringBuilder();
536-
537533
//Replace all the items in the map with canonicalized versions.
538534

539535
Set<UriSegment> set = parseMap.keySet();
@@ -542,8 +538,7 @@ public String getCanonicalizedURI(URI dirtyUri) throws IntrusionException{
542538
boolean allowMixed = sg.getBooleanProp("Encoder.AllowMixedEncoding");
543539
boolean allowMultiple = sg.getBooleanProp("Encoder.AllowMultipleEncoding");
544540
for(UriSegment seg: set){
545-
String value = canonicalize(parseMap.get(seg), allowMultiple, allowMixed);
546-
value = value == null ? "" : value;
541+
String value = "";
547542
//In the case of a uri query, we need to break up and canonicalize the internal parts of the query.
548543
if(seg == UriSegment.QUERY && null != parseMap.get(seg)){
549544
StringBuilder qBuilder = new StringBuilder();
@@ -571,6 +566,10 @@ public String getCanonicalizedURI(URI dirtyUri) throws IntrusionException{
571566
} catch (UnsupportedEncodingException e) {
572567
logger.debug(Logger.EVENT_FAILURE, "decoding error when parsing [" + dirtyUri.toString() + "]");
573568
}
569+
} else {
570+
String extractedInput = parseMap.get(seg);
571+
value = canonicalize(extractedInput, allowMultiple, allowMixed);
572+
value = value == null ? "" : value;
574573
}
575574
//Check if the port is -1, if it is, omit it from the output.
576575
if(seg == UriSegment.PORT){
@@ -592,11 +591,16 @@ public String getCanonicalizedURI(URI dirtyUri) throws IntrusionException{
592591
*/
593592
protected String buildUrl(Map<UriSegment, String> parseMap){
594593
StringBuilder sb = new StringBuilder();
595-
sb.append(parseMap.get(UriSegment.SCHEME))
596-
.append("://")
594+
boolean schemePresent = parseMap.get(UriSegment.SCHEME).equals("") ? false : true;
595+
596+
if(schemePresent) {
597+
sb.append(parseMap.get(UriSegment.SCHEME))
598+
.append("://");
599+
}
600+
597601
//can't use SCHEMESPECIFICPART for this, because we need to canonicalize all the parts of the query.
598602
//USERINFO is also deprecated. So we technically have more than we need.
599-
.append(parseMap.get(UriSegment.AUTHORITY) == null || parseMap.get(UriSegment.AUTHORITY).equals("") ? "" : parseMap.get(UriSegment.AUTHORITY))
603+
sb.append(parseMap.get(UriSegment.AUTHORITY) == null || parseMap.get(UriSegment.AUTHORITY).equals("") ? "" : parseMap.get(UriSegment.AUTHORITY))
600604
.append(parseMap.get(UriSegment.PATH) == null || parseMap.get(UriSegment.PATH).equals("") ? "" : parseMap.get(UriSegment.PATH))
601605
.append(parseMap.get(UriSegment.QUERY) == null || parseMap.get(UriSegment.QUERY).equals("")
602606
? "" : "?" + parseMap.get(UriSegment.QUERY))

‎src/test/java/org/owasp/esapi/codecs/HTMLEntityCodecTest.java‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22

33
import static org.junit.Assert.assertEquals;
44

5+
import org.junit.Ignore;
56
import org.junit.Test;
67

78
public class HTMLEntityCodecTest {
@@ -48,4 +49,23 @@ public void testMixedBmpAndNonBmp(){
4849
String input = bmp + nonBMP;
4950
assertEquals(expected, codec.encode(new char[0], input));
5051
}
52+
53+
@Test
54+
/**
55+
* TODO: The following methods are unit tests I'm checking in for an issue to be worked and fixed.
56+
*/
57+
@Ignore("Pre check-in for issue #827")
58+
public void testIssue827() {
59+
String input = "/webapp/ux/home?d=1705914006565&status=login&ticket=1705914090394_HzJpTROVfhW-JhRW0OqDbHu7tWXXlgrKSUmOzIMsZNCcUIiYGMXX_Q%3D%3D&newsess=false&roleid=DP010101/0007&origin=ourprogram";
60+
String expected = input;
61+
assertEquals(expected, codec.decode(input));
62+
}
63+
64+
@Test
65+
@Ignore("Pre check-in for issue #827")
66+
public void testIssue827OnlyOR() {
67+
String input = "&origin=ourprogram";
68+
String expected = input;
69+
assertEquals(expected, codec.decode(input));
70+
}
5171
}

‎src/test/java/org/owasp/esapi/reference/EncoderTest.java‎

Lines changed: 84 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -15,25 +15,21 @@
1515
*/
1616
package org.owasp.esapi.reference;
1717

18-
import static org.junit.Assert.assertEquals;
1918
import static org.junit.Assert.assertNotEquals;
2019

2120
import java.io.IOException;
2221
import java.io.UnsupportedEncodingException;
2322
import java.net.URI;
24-
import java.util.List;
2523
import java.util.ArrayList;
2624
import java.util.Arrays;
27-
import java.util.HashMap;
28-
import java.util.Map;
29-
import java.util.Map.Entry;
30-
import java.util.regex.Matcher;
31-
import java.util.regex.Pattern;
25+
import java.util.List;
3226

27+
import org.junit.Ignore;
3328
import org.owasp.esapi.ESAPI;
3429
import org.owasp.esapi.Encoder;
3530
import org.owasp.esapi.EncoderConstants;
36-
import org.owasp.esapi.codecs.CSSCodec;
31+
import org.owasp.esapi.SecurityConfiguration;
32+
import org.owasp.esapi.SecurityConfigurationWrapper;
3733
import org.owasp.esapi.codecs.Codec;
3834
import org.owasp.esapi.codecs.HTMLEntityCodec;
3935
import org.owasp.esapi.codecs.MySQLCodec;
@@ -43,8 +39,6 @@
4339
import org.owasp.esapi.codecs.WindowsCodec;
4440
import org.owasp.esapi.errors.EncodingException;
4541
import org.owasp.esapi.errors.IntrusionException;
46-
import org.owasp.esapi.SecurityConfiguration;
47-
import org.owasp.esapi.SecurityConfigurationWrapper;
4842

4943
import junit.framework.Test;
5044
import junit.framework.TestCase;
@@ -678,6 +672,7 @@ public void testDecodeFromURL() throws Exception {
678672
fail();
679673
}
680674
try {
675+
//FIXME: Rewrite this to use expected Exceptions.
681676
instance.decodeFromURL( "%3xridiculous" );
682677
fail();
683678
} catch( Exception e ) {
@@ -916,6 +911,50 @@ public void testGetCanonicalizedUri() throws Exception {
916911
assertEquals(expectedUri, e.getCanonicalizedURI(uri));
917912

918913
}
914+
915+
public void testGetCanonicalizedUriWithAnHTMLEntityCollision() throws Exception {
916+
System.out.println("GetCanonicalizedUriWithAnHTMLEntityCollision");
917+
Encoder e = ESAPI.encoder();
918+
919+
String expectedUri = "http://[email protected]/path_to/resource?foo=bar&para1=test";
920+
//Please note that section 3.2.1 of RFC-3986 explicitly states not to encode
921+
//password information as in http://palpatine:[email protected], and this will
922+
//not appear in the userinfo field.
923+
String input = "http://[email protected]/path_to/resource?foo=bar&para1=test";
924+
URI uri = new URI(input);
925+
System.out.println(uri.toString());
926+
assertEquals(expectedUri, e.getCanonicalizedURI(uri));
927+
928+
}
929+
930+
@org.junit.Ignore("Pre-check in unit test for issue #826")
931+
public void Issue826GetCanonicalizedUriWithMultipleEncoding() throws Exception {
932+
System.out.println("GetCanonicalizedUriWithAnHTMLEntityCollision");
933+
Encoder e = ESAPI.encoder();
934+
String expectedUri = "http://[email protected]/path_to/resource?foo=bar&para1=&amp;amp;amp;test";
935+
//Please note that section 3.2.1 of RFC-3986 explicitly states not to encode
936+
//password information as in http://palpatine:[email protected], and this will
937+
//not appear in the userinfo field.
938+
String input = "http://[email protected]/path_to/resource?foo=bar&para1=&amp;amp;amp;test";
939+
URI uri = new URI(input);
940+
System.out.println(uri.toString());
941+
assertEquals(expectedUri, e.getCanonicalizedURI(uri));
942+
943+
}
944+
public void testGetCanonicalizedUriWithMultQueryParams() throws Exception {
945+
System.out.println("getCanonicalizedUri");
946+
Encoder e = ESAPI.encoder();
947+
948+
String expectedUri = "http://palpatine@foo bar.com/path_to/resource?foo=bar&bar=foo#frag";
949+
//Please note that section 3.2.1 of RFC-3986 explicitly states not to encode
950+
//password information as in http://palpatine:[email protected], and this will
951+
//not appear in the userinfo field.
952+
String input = "http://palpatine@foo%20bar.com/path_to/resource?foo=bar&bar=foo#frag";
953+
URI uri = new URI(input);
954+
System.out.println(uri.toString());
955+
assertEquals(expectedUri, e.getCanonicalizedURI(uri));
956+
957+
}
919958

920959
public void testGetCanonicalizedUriPiazza() throws Exception {
921960
System.out.println("getCanonicalizedUriPiazza");
@@ -931,6 +970,41 @@ public void testGetCanonicalizedUriPiazza() throws Exception {
931970
assertEquals(expectedUri, e.getCanonicalizedURI(uri));
932971

933972
}
973+
974+
public void testIssue824() throws Exception {
975+
System.out.println("getCanonicalizedUriPiazza");
976+
Encoder e = ESAPI.encoder();
977+
978+
String expectedUri = "/webapp/ux/home?d=1705914006565&status=login&ticket=1705914090394_HzJpTROVfhW-JhRW0OqDbHu7tWXXlgrKSUmOzIMsZNCcUIiYGMXX_Q==&newsess=false&roleid=DP010101/0007&origin=ourprogram";
979+
//Please note that section 3.2.1 of RFC-3986 explicitly states not to encode
980+
//password information as in http://palpatine:[email protected], and this will
981+
//not appear in the userinfo field.
982+
String input = "/webapp/ux/home?d=1705914006565&status=login&ticket=1705914090394_HzJpTROVfhW-JhRW0OqDbHu7tWXXlgrKSUmOzIMsZNCcUIiYGMXX_Q%3D%3D&newsess=false&roleid=DP010101/0007&origin=ourprogram";
983+
URI uri = new URI(input);
984+
System.out.println(uri.toString());
985+
assertEquals(expectedUri, e.getCanonicalizedURI(uri));
986+
987+
}
988+
989+
@org.junit.Ignore("Pre-check in unit test for issue #826")
990+
public void Issue826GetCanonicalizedDoubleAmpersand() throws Exception {
991+
System.out.println("getCanonicalizedDoubleAmpersand");
992+
Encoder e = ESAPI.encoder();
993+
String expectedUri = "http://127.0.0.1:3000/campaigns?goal=all&section=active&sort-by=-id&status=Draft%2C&html=&amp;contentLaunched";
994+
//http://127.0.0.1:3000/campaigns?goal=all&section=active&sort-by=-id&status=Draft,&html=null&=null&amp;contentLaunched=null
995+
/*
996+
* In this case, the URI class should break up the HTML entity in the query so
997+
*/
998+
String input = "http://127.0.0.1:3000/campaigns?goal=all&section=active&sort-by=-id&status=Draft%2C&html=&&amp;contentLaunched";
999+
URI uri = new URI(input);
1000+
System.out.println(uri.toString());
1001+
try {
1002+
assertEquals(expectedUri, e.getCanonicalizedURI(uri));
1003+
fail();
1004+
} catch (Exception ex) {
1005+
//Expected
1006+
}
1007+
}
9341008

9351009
public void testGetCanonicalizedUriWithMailto() throws Exception {
9361010
System.out.println("getCanonicalizedUriWithMailto");

0 commit comments

Comments
 (0)