---snip---
> ...but there might be security/implementation
> issues that I missed.
---snip---
Oops! Found one big issue right after I sent off the patch. Patch now checks
for non-hex-character-strings in the encoding, and returns null if present
( similar to what happens if a control character is in the encoded string,
or if the url tries to access resources out of the context etc... ). Let's
only hope I don't find yet another error in my patch right after I send this
one out!
David Weinrich
--- DefaultServlet.java Thu Dec 28 00:30:23 2000
+++ DefaultServletEd.java Thu Dec 28 00:29:51 2000
@@ -729,9 +729,41 @@
* @param path Path to be normalized
*/
protected String normalize(String path) {
+ String normalized = path;
+
+ // Resolve encoded characters in the normalized path,
+ // which also handles encoded spaces so we can skip that later.
+ // Placed at the beginning of the chain so that encoded
+ // bad stuff(tm) can be caught by the later checks
+ while (true) {
+ int index = normalized.indexOf("%");
+ if (index < 0)
+ break;
+ char replaceChar;
+ try {
+ replaceChar = (char) (
+ Short.parseShort(
+ normalized.substring( index + 1, index + 3 ), 16
+ )
+ );
+ } catch ( NumberFormatException nfe ) {
+ return (null); // bad encoded characters in url
+ }
+ // check for control characters ( values 00-1f and 7f-9f),
+ // return null if present. See:
+ // http://www.unicode.org/charts/PDF/U0000.pdf
+ // http://www.unicode.org/charts/PDF/U0080.pdf
+ if ( Character.isISOControl( replaceChar ) ) {
+ return (null);
+ }
+ normalized = normalized.substring(0, index) +
+ replaceChar +
+ normalized.substring(index + 3);
+ }
+
+
// Normalize the slashes and add leading slash if necessary
- String normalized = path;
if (normalized.indexOf('\\') >= 0)
normalized = normalized.replace('\\', '/');
if (!normalized.startsWith("/"))
@@ -745,15 +777,6 @@
normalized = normalized.substring(0, index) +
normalized.substring(index + 1);
}
-
- // Resolve occurrences of "%20" in the normalized path
- while (true) {
- int index = normalized.indexOf("%20");
- if (index < 0)
- break;
- normalized = normalized.substring(0, index) + " " +
- normalized.substring(index + 3);
- }
// Resolve occurrences of "/./" in the normalized path
while (true) {
--- ResourcesBase.java Thu Dec 28 00:32:58 2000
+++ ResourcesBaseEd.java Thu Dec 28 00:08:15 2000
@@ -961,9 +961,39 @@
* @param path Path to be normalized
*/
protected String normalize(String path) {
+ String normalized = path;
+
+ // Resolve encoded characters in the normalized path,
+ // which also handles encoded spaces so we can skip that later.
+ // Placed at the beginning of the chain so that encoded
+ // bad stuff(tm) can be caught by the later checks
+ while (true) {
+ int index = normalized.indexOf("%");
+ if (index < 0)
+ break;
+ char replaceChar;
+ try {
+ replaceChar = (char) (
+ Short.parseShort(
+ normalized.substring( index + 1, index + 3 ), 16
+ )
+ );
+ } catch ( NumberFormatException nfe ) {
+ return (null); // bad encoded characters in url
+ }
+ // check for control characters ( values 00-1f and 7f-9f),
+ // return null if present. See:
+ // http://www.unicode.org/charts/PDF/U0000.pdf
+ // http://www.unicode.org/charts/PDF/U0080.pdf
+ if ( Character.isISOControl( replaceChar ) ) {
+ return (null);
+ }
+ normalized = normalized.substring(0, index) +
+ replaceChar +
+ normalized.substring(index + 3);
+ }
// Normalize the slashes and add leading slash if necessary
- String normalized = path;
if (normalized.indexOf('\\') >= 0)
normalized = normalized.replace('\\', '/');
if (!normalized.startsWith("/"))
@@ -977,15 +1007,6 @@
normalized = normalized.substring(0, index) +
normalized.substring(index + 1);
}
-
- // Resolve occurrences of "%20" in the normalized path
- while (true) {
- int index = normalized.indexOf("%20");
- if (index < 0)
- break;
- normalized = normalized.substring(0, index) + " " +
- normalized.substring(index + 3);
- }
// Resolve occurrences of "/./" in the normalized path
while (true) {
