costin 01/07/15 17:12:04
Modified: src/share/org/apache/tomcat/modules/mappers
DecodeInterceptor.java
Log:
Major security addition - DecodeInterceptor is now able to "normalize"
a URI, using the same (well tested ) alghoritm that apache does.
This will remove all unsafe path components at the earliest stage,
making sure all components benefit from this ( not only StaticInterceptor
via FileUtils ).
We deal with //, /./, /../, /.., /.
Note that this behavior can be disabled, if you think following
ad-literam the servlet spec is better than security. The servlet spec
requires the user to get the "original" URI - which this fix obviously disables.
On the other side, not doing the normalization opens _huge_ security problems,
and _all_ servlets that are using paths ( a default servlet, etc) will
have to know how to secure the path. Since the container still has problems,
it's very unlikely most user code will be able to do so.
In particular the security matching is greatly affected.
Revision Changes Path
1.4 +136 -0
jakarta-tomcat/src/share/org/apache/tomcat/modules/mappers/DecodeInterceptor.java
Index: DecodeInterceptor.java
===================================================================
RCS file:
/home/cvs/jakarta-tomcat/src/share/org/apache/tomcat/modules/mappers/DecodeInterceptor.java,v
retrieving revision 1.3
retrieving revision 1.4
diff -u -r1.3 -r1.4
--- DecodeInterceptor.java 2001/06/16 22:05:05 1.3
+++ DecodeInterceptor.java 2001/07/16 00:12:03 1.4
@@ -81,6 +81,7 @@
private int encodingInfoNote;
private int sessionEncodingNote;
+ private boolean safe=true;
public DecodeInterceptor() {
}
@@ -103,6 +104,14 @@
charsetAttribute=s;
charsetURIAttribute=";" + charsetAttribute + "=";
}
+
+ /** Decode interceptor can normalize unsafe urls, by eliminating
+ dangerous things like /../, // , etc - all of them are known
+ as very dangerous for security.
+ */
+ public void setSafe( boolean b ) {
+ safe=b;
+ }
/* -------------------- Initialization -------------------- */
@@ -116,6 +125,123 @@
}
/* -------------------- Request mapping -------------------- */
+
+ // Based on Apache's path normalization code
+ private void normalizePath(MessageBytes pathMB ) {
+ if( debug> 0 ) log( "Normalize " + pathMB.toString());
+ if( pathMB.getType() != MessageBytes.T_BYTES ) return;
+
+ ByteChunk bc=pathMB.getByteChunk();
+
+ int start=bc.getStart();
+ int end=bc.getEnd();
+ byte buff[]=bc.getBytes();
+ int i=0;
+ int j=0;
+
+ // remove //
+ for( i=start, j=start; i<end-1; i++ ) {
+ if( buff[i]== '/' && buff[i+1]=='/' ) {
+ while( buff[i+1]=='/' ) i++;
+ }
+ buff[j++]=buff[i];
+ }
+ if( i!=j ) {
+ buff[j++]=buff[end-1];
+ end=j;
+ bc.setEnd( end );
+ pathMB.resetStringValue();
+ if( debug > 0 ) {
+ log( "Eliminate // " + pathMB.toString() + " " + start + " " + end );
+ }
+ }
+
+ // remove /./
+ for( i=start, j=start; i<end-1; i++ ) {
+ if( buff[i]== '.' && buff[i+1]=='/' &&
+ ( i==0 || buff[i-1]=='/' )) {
+ // "/./"
+ i+=1;
+ if( i==end-1 ) j--; // cut the ending /
+ } else {
+ buff[j++]=buff[i];
+ }
+ }
+ if( i!=j ) {
+ buff[j++]=buff[end-1];
+ end=j;
+ bc.setEnd( end );
+ pathMB.resetStringValue();
+ if( debug > 0 ) {
+ log( "Eliminate /./ " + pathMB.toString());
+ }
+ }
+
+ // remove /. at the end
+ j=end;
+ if( end==start+1 && buff[start]== '.' )
+ end--;
+ else if( end > start+1 && buff[ end-1 ] == '.' &&
+ buff[end-2]=='/' ) {
+ end=end-2;
+ }
+ if( end!=j ) {
+ bc.setEnd( end );
+ pathMB.resetStringValue();
+ if( debug > 0 ) {
+ log( "Eliminate ending /. " + pathMB.toString());
+ }
+ }
+
+ // remove /../
+ for( i=start, j=start; i<end-2; i++ ) {
+ if( buff[i] == '.' &&
+ buff[i+1] == '.' &&
+ buff[i+2]== '/' &&
+ ( i==0 || buff[ i-1 ] == '/' ) ) {
+
+ i+=1;
+ // look for the previous /
+ j=j-2;
+ while( j>0 && buff[j]!='/' ) {
+ j--;
+ }
+ } else {
+ buff[j++]=buff[i];
+ }
+ }
+ if( i!=j ) {
+ buff[j++]=buff[end-2];
+ buff[j++]=buff[end-1];
+ end=j;
+ bc.setEnd( end );
+ pathMB.resetStringValue();
+ if( debug > 0 ) {
+ log( "Eliminate /../ " + pathMB.toString());
+ }
+ }
+
+
+ // remove trailing xx/..
+ j=end;
+ if( end>start + 3 &&
+ buff[end-1]=='.' &&
+ buff[end-2]=='.' &&
+ buff[end-3]=='/' ) {
+ end-=4;
+ while( end>0 && buff[end]!='/' )
+ end--;
+ }
+ if( end!=j ) {
+ bc.setEnd( end );
+ pathMB.resetStringValue();
+ if( debug > 0 ) {
+ log( "Eliminate ending /.. " + pathMB.toString());
+ }
+ }
+
+ }
+
public int postReadRequest( Request req ) {
MessageBytes pathMB = req.requestURI();
// copy the request
@@ -123,6 +249,16 @@
if( pathMB.isNull())
throw new RuntimeException("ASSERT: null path in request URI");
+ if( safe &&
+ ( pathMB.indexOf("//") >= 0 ||
+ pathMB.indexOf("/." ) >=0
+ )) {
+ //debug=1;
+ normalizePath( pathMB );
+ if( debug > 0 )
+ log( "Normalized url " + pathMB );
+ }
+
//if( path.indexOf("?") >=0 )
// throw new RuntimeException("ASSERT: ? in requestURI");
