Author: jim Date: Sun Sep 18 18:34:44 2011 New Revision: 1172317 URL: http://svn.apache.org/viewvc?rev=1172317&view=rev Log: Detect incomplete AJP messages
Modified: tomcat/tc5.5.x/trunk/STATUS.txt tomcat/tc5.5.x/trunk/connectors/jk/java/org/apache/coyote/ajp/AjpAprProcessor.java tomcat/tc5.5.x/trunk/connectors/jk/java/org/apache/coyote/ajp/AjpMessage.java tomcat/tc5.5.x/trunk/connectors/jk/java/org/apache/coyote/ajp/LocalStrings.properties Modified: tomcat/tc5.5.x/trunk/STATUS.txt URL: http://svn.apache.org/viewvc/tomcat/tc5.5.x/trunk/STATUS.txt?rev=1172317&r1=1172316&r2=1172317&view=diff ============================================================================== --- tomcat/tc5.5.x/trunk/STATUS.txt (original) +++ tomcat/tc5.5.x/trunk/STATUS.txt Sun Sep 18 18:34:44 2011 @@ -24,14 +24,6 @@ $Id$ PATCHES ACCEPTED TO BACKPORT FROM TRUNK/OTHER: [ start all new proposals below, under PATCHES PROPOSED. ] -* Detect incomplete AJP messages and reject the associated request if one is - found - http://people.apache.org/~markt/patches/2011-08-25-ajp-incomplete-msg-tc5.patch - +1: markt - +1: kkolinko, rjung: In AjpMessage#validatePos() s/"" + posToTest/String.valueOf(posToTest)/ - +1 to mark AjpMessage#getBytes(byte[]) as /**@deprecated*/ - it is never used. - -1: - PATCHES PROPOSED TO BACKPORT: [ New proposals should be added at the end of the list ] Modified: tomcat/tc5.5.x/trunk/connectors/jk/java/org/apache/coyote/ajp/AjpAprProcessor.java URL: http://svn.apache.org/viewvc/tomcat/tc5.5.x/trunk/connectors/jk/java/org/apache/coyote/ajp/AjpAprProcessor.java?rev=1172317&r1=1172316&r2=1172317&view=diff ============================================================================== --- tomcat/tc5.5.x/trunk/connectors/jk/java/org/apache/coyote/ajp/AjpAprProcessor.java (original) +++ tomcat/tc5.5.x/trunk/connectors/jk/java/org/apache/coyote/ajp/AjpAprProcessor.java Sun Sep 18 18:34:44 2011 @@ -1136,7 +1136,7 @@ public class AjpAprProcessor implements return false; } - bodyMessage.getBytes(bodyBytes); + bodyMessage.getBodyBytes(bodyBytes); empty = false; return true; } Modified: tomcat/tc5.5.x/trunk/connectors/jk/java/org/apache/coyote/ajp/AjpMessage.java URL: http://svn.apache.org/viewvc/tomcat/tc5.5.x/trunk/connectors/jk/java/org/apache/coyote/ajp/AjpMessage.java?rev=1172317&r1=1172316&r2=1172317&view=diff ============================================================================== --- tomcat/tc5.5.x/trunk/connectors/jk/java/org/apache/coyote/ajp/AjpMessage.java (original) +++ tomcat/tc5.5.x/trunk/connectors/jk/java/org/apache/coyote/ajp/AjpMessage.java Sun Sep 18 18:34:44 2011 @@ -297,11 +297,13 @@ public class AjpMessage { public int getInt() { int b1 = buf[pos++] & 0xFF; int b2 = buf[pos++] & 0xFF; + validatePos(pos); return (b1<<8) + b2; } public int peekInt() { + validatePos(pos + 2); int b1 = buf[pos] & 0xFF; int b2 = buf[pos+1] & 0xFF; return (b1<<8) + b2; @@ -310,25 +312,41 @@ public class AjpMessage { public byte getByte() { byte res = buf[pos++]; + validatePos(pos); return res; } public byte peekByte() { + validatePos(pos + 1); byte res = buf[pos]; return res; } - public void getBytes(MessageBytes mb) { + doGetBytes(mb, true); + } + + public void getBodyBytes(MessageBytes mb) { + doGetBytes(mb, false); + } + + private void doGetBytes(MessageBytes mb, boolean terminated) { int length = getInt(); if ((length == 0xFFFF) || (length == -1)) { mb.recycle(); return; } + if (terminated) { + validatePos(pos + length + 1); + } else { + validatePos(pos + length); + } mb.setBytes(buf, pos, length); pos += length; - pos++; // Skip the terminating \0 + if (terminated) { + pos++; // Skip the terminating \0 + } } @@ -338,6 +356,7 @@ public class AjpMessage { * on the encoding. * * @return The number of bytes copied. + * @deprecated */ public int getBytes(byte[] dest) { int length = getInt(); @@ -349,6 +368,7 @@ public class AjpMessage { if ((length == 0xFFFF) || (length == -1)) { return 0; } + validatePos(pos + length + 1); System.arraycopy(buf, pos, dest, 0, length); pos += length; @@ -371,6 +391,7 @@ public class AjpMessage { b1 |= (buf[pos++] & 0xFF); b1 <<=8; b1 |= (buf[pos++] & 0xFF); + validatePos(pos); return b1; } @@ -419,6 +440,15 @@ public class AjpMessage { } + private void validatePos(int posToTest) { + if (posToTest > len + 4) { + // Trying to read data beyond the end of the AJP message + throw new ArrayIndexOutOfBoundsException(sm.getString( + "ajpMessage.invalidPos", String.valueOf(posToTest))); + } + } + + // ------------------------------------------------------ Protected Methods Modified: tomcat/tc5.5.x/trunk/connectors/jk/java/org/apache/coyote/ajp/LocalStrings.properties URL: http://svn.apache.org/viewvc/tomcat/tc5.5.x/trunk/connectors/jk/java/org/apache/coyote/ajp/LocalStrings.properties?rev=1172317&r1=1172316&r2=1172317&view=diff ============================================================================== --- tomcat/tc5.5.x/trunk/connectors/jk/java/org/apache/coyote/ajp/LocalStrings.properties (original) +++ tomcat/tc5.5.x/trunk/connectors/jk/java/org/apache/coyote/ajp/LocalStrings.properties Sun Sep 18 18:34:44 2011 @@ -49,4 +49,4 @@ ajpmessage.null=Cannot append null value ajpmessage.overflow=Overflow error for buffer adding {0} bytes at position {1} ajpmessage.read=Requested {0} bytes exceeds message available data ajpmessage.invalid=Invalid message recieved with signature {0} - +ajpMessage.invalidPos=Requested read of bytes at position [{0}] which is beyond the end of the AJP message --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org