You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by ma...@apache.org on 2011/09/02 13:49:53 UTC
svn commit: r1164490 - in /tomcat/tc6.0.x/trunk: ./
java/org/apache/coyote/ajp/ webapps/docs/
Author: markt
Date: Fri Sep 2 11:49:53 2011
New Revision: 1164490
URL: http://svn.apache.org/viewvc?rev=1164490&view=rev
Log:
Detect incomplete AJP messages and reject the associated request if one is found
Modified:
tomcat/tc6.0.x/trunk/STATUS.txt
tomcat/tc6.0.x/trunk/java/org/apache/coyote/ajp/AjpAprProcessor.java
tomcat/tc6.0.x/trunk/java/org/apache/coyote/ajp/AjpMessage.java
tomcat/tc6.0.x/trunk/java/org/apache/coyote/ajp/AjpProcessor.java
tomcat/tc6.0.x/trunk/java/org/apache/coyote/ajp/LocalStrings.properties
tomcat/tc6.0.x/trunk/webapps/docs/changelog.xml
Modified: tomcat/tc6.0.x/trunk/STATUS.txt
URL: http://svn.apache.org/viewvc/tomcat/tc6.0.x/trunk/STATUS.txt?rev=1164490&r1=1164489&r2=1164490&view=diff
==============================================================================
--- tomcat/tc6.0.x/trunk/STATUS.txt (original)
+++ tomcat/tc6.0.x/trunk/STATUS.txt Fri Sep 2 11:49:53 2011
@@ -89,15 +89,3 @@ PATCHES PROPOSED TO BACKPORT:
https://issues.apache.org/bugzilla/attachment.cgi?id=27434
+1: markt, kkolinko
-1:
-
-* 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-tc6.patch
- +1: markt, kfujino
- +1: kkolinko:
- Need to s/"" + pos/String.valueOf(posToTest)/ in AjpMessage#validatePos()
- - wrong variable is used.
- +1 to annotate AjpMessage#getBytes(byte[]) as @Deprecated - it is
- never used, so cannot say whether consuming terminating '\0' is
- needed like in #doGetBytes().
- -1:
Modified: tomcat/tc6.0.x/trunk/java/org/apache/coyote/ajp/AjpAprProcessor.java
URL: http://svn.apache.org/viewvc/tomcat/tc6.0.x/trunk/java/org/apache/coyote/ajp/AjpAprProcessor.java?rev=1164490&r1=1164489&r2=1164490&view=diff
==============================================================================
--- tomcat/tc6.0.x/trunk/java/org/apache/coyote/ajp/AjpAprProcessor.java (original)
+++ tomcat/tc6.0.x/trunk/java/org/apache/coyote/ajp/AjpAprProcessor.java Fri Sep 2 11:49:53 2011
@@ -1161,7 +1161,7 @@ public class AjpAprProcessor implements
return false;
}
- bodyMessage.getBytes(bodyBytes);
+ bodyMessage.getBodyBytes(bodyBytes);
empty = false;
return true;
}
Modified: tomcat/tc6.0.x/trunk/java/org/apache/coyote/ajp/AjpMessage.java
URL: http://svn.apache.org/viewvc/tomcat/tc6.0.x/trunk/java/org/apache/coyote/ajp/AjpMessage.java?rev=1164490&r1=1164489&r2=1164490&view=diff
==============================================================================
--- tomcat/tc6.0.x/trunk/java/org/apache/coyote/ajp/AjpMessage.java (original)
+++ tomcat/tc6.0.x/trunk/java/org/apache/coyote/ajp/AjpMessage.java Fri Sep 2 11:49:53 2011
@@ -300,11 +300,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;
@@ -313,25 +315,42 @@ 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) {
- int length = getInt();
+ 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
+ }
}
@@ -341,7 +360,10 @@ public class AjpMessage {
* on the encoding.
*
* @return The number of bytes copied.
+ *
+ * @deprecated
*/
+ @Deprecated
public int getBytes(byte[] dest) {
int length = getInt();
if (pos + length > buf.length) {
@@ -352,6 +374,7 @@ public class AjpMessage {
if ((length == 0xFFFF) || (length == -1)) {
return 0;
}
+ validatePos(pos + length + 1);
System.arraycopy(buf, pos, dest, 0, length);
pos += length;
@@ -374,6 +397,7 @@ public class AjpMessage {
b1 |= (buf[pos++] & 0xFF);
b1 <<=8;
b1 |= (buf[pos++] & 0xFF);
+ validatePos(pos);
return b1;
}
@@ -427,6 +451,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/tc6.0.x/trunk/java/org/apache/coyote/ajp/AjpProcessor.java
URL: http://svn.apache.org/viewvc/tomcat/tc6.0.x/trunk/java/org/apache/coyote/ajp/AjpProcessor.java?rev=1164490&r1=1164489&r2=1164490&view=diff
==============================================================================
--- tomcat/tc6.0.x/trunk/java/org/apache/coyote/ajp/AjpProcessor.java (original)
+++ tomcat/tc6.0.x/trunk/java/org/apache/coyote/ajp/AjpProcessor.java Fri Sep 2 11:49:53 2011
@@ -1118,7 +1118,7 @@ public class AjpProcessor implements Act
return false;
}
- bodyMessage.getBytes(bodyBytes);
+ bodyMessage.getBodyBytes(bodyBytes);
empty = false;
return true;
}
Modified: tomcat/tc6.0.x/trunk/java/org/apache/coyote/ajp/LocalStrings.properties
URL: http://svn.apache.org/viewvc/tomcat/tc6.0.x/trunk/java/org/apache/coyote/ajp/LocalStrings.properties?rev=1164490&r1=1164489&r2=1164490&view=diff
==============================================================================
--- tomcat/tc6.0.x/trunk/java/org/apache/coyote/ajp/LocalStrings.properties (original)
+++ tomcat/tc6.0.x/trunk/java/org/apache/coyote/ajp/LocalStrings.properties Fri Sep 2 11:49:53 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
Modified: tomcat/tc6.0.x/trunk/webapps/docs/changelog.xml
URL: http://svn.apache.org/viewvc/tomcat/tc6.0.x/trunk/webapps/docs/changelog.xml?rev=1164490&r1=1164489&r2=1164490&view=diff
==============================================================================
--- tomcat/tc6.0.x/trunk/webapps/docs/changelog.xml (original)
+++ tomcat/tc6.0.x/trunk/webapps/docs/changelog.xml Fri Sep 2 11:49:53 2011
@@ -63,6 +63,10 @@
<bug>51698</bug>: Fix CVE-2011-3190. Prevent AJP message injection.
(markt)
</fix>
+ <fix>
+ Detect incomplete AJP messages and reject the associated request if one
+ is found. (markt)
+ </fix>
</changelog>
</subsection>
<subsection name="Jasper">
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org