serverscanning/capnproto-java-rpc
16
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

Merge remote-tracking branch 'sandstorm/master' into vaci/merge2

Browse source
This commit is contained in:
Vaci Koblizek 2021-11-09 21:48:36 +00:00
commit 9a32482541
16 changed files with 355 additions and 27 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

12
CHANGELOG.md
View file

@ -1,2 +1,14 @@
## v0.1.13
- Improved robustness to StructPointer integer wrapping problems.
## v0.1.12
- Fix bug in StructPointer.wordSize() found by Martin Dindoffer.
## v0.1.11
- Fix memory and cpu amplification vulnerabilities found by Martin Dindoffer.
## v0.1.10
- Make even more compatible with Java 8.
## v0.1.9
- Add `<maven.compiler.release>8</maven.compiler.release>` to pom.xml, to increase compability with Java 8.

4
benchmark/pom.xml
View file

@ -5,7 +5,7 @@
<artifactId>benchmark</artifactId>
<packaging>jar</packaging>
<description>capnproto-java benchmark</description>
<version>0.1.10-SNAPSHOT</version>
<version>0.1.14-SNAPSHOT</version>
<name>capnproto-java benchmark</name>
<organization>
<name>org.capnproto</name>
@ -39,7 +39,7 @@
<dependency>
<groupId>org.capnproto</groupId>
<artifactId>runtime</artifactId>
<version>0.1.10-SNAPSHOT</version>
<version>0.1.14-SNAPSHOT</version>
</dependency>
</dependencies>

4
compiler/pom.xml
View file

@ -5,7 +5,7 @@
<artifactId>compiler</artifactId>
<packaging>jar</packaging>
<description>schema compiler plugin for java</description>
<version>0.1.10-SNAPSHOT</version>
<version>0.1.14-SNAPSHOT</version>
<name>capnpc-java</name>
<organization>
<name>org.capnproto</name>
@ -44,7 +44,7 @@
<dependency>
<groupId>org.capnproto</groupId>
<artifactId>runtime</artifactId>
<version>0.1.10-SNAPSHOT</version>
<version>0.1.14-SNAPSHOT</version>
</dependency>
</dependencies>

54
compiler/src/test/java/org/capnproto/test/EncodingTest.java
View file

@ -615,6 +615,24 @@ public class EncodingTest {
root.getAnyPointerField().getAs(StructList.newFactory(Test.TestAllTypes.factory));
}
// Test that we throw an exception on out-of-bounds list pointers.
// Before v0.1.11, we were vulnerable to a cpu amplification attack:
// reading an out-of-bounds pointer to list a huge number of elements of size BIT,
// when read as a struct list, would return without error.
@org.junit.Test(expected=DecodeException.class)
public void testListPointerOutOfBounds() throws DecodeException {
byte[] bytes = new byte[]
{0,0,0,0, 0,0,1,0, // struct, one pointer
1, 0x2f, 0, 0, 1, 0, -127, -128}; // list, points out of bounds.
ByteBuffer segment = ByteBuffer.wrap(bytes);
segment.order(ByteOrder.LITTLE_ENDIAN);
MessageReader message = new MessageReader(new ByteBuffer[]{segment},
ReaderOptions.DEFAULT_READER_OPTIONS);
Test.TestAnyPointer.Reader root = message.getRoot(Test.TestAnyPointer.factory);
root.getAnyPointerField().getAs(StructList.newFactory(Test.TestAllTypes.factory));
}
@org.junit.Test
public void testLongUint8List() {
MessageBuilder message = new MessageBuilder();
@ -864,5 +882,41 @@ public class EncodingTest {
root2.setAs(AnyPointer.factory, message1.getRoot(AnyPointer.factory).asReader());
TestUtil.checkTestMessage(root2.getAs(Test.TestAllTypes.factory));
}
@org.junit.Test
public void testZeroPointerUnderflow() throws DecodeException {
byte[] bytes = new byte[8 + 8 * 65535];
bytes[4] = -1;
bytes[5] = -1; // struct pointer with 65535 words of data section.
for (int ii = 0; ii < 8 * 65535; ++ii) {
bytes[8 + ii] = 101; // populate the data section with sentinel data
}
ByteBuffer segment = ByteBuffer.wrap(bytes);
segment.order(ByteOrder.LITTLE_ENDIAN);
MessageReader message1 = new MessageReader(new ByteBuffer[]{segment},
ReaderOptions.DEFAULT_READER_OPTIONS);
Test.TestAnyPointer.Reader message1RootReader = message1.getRoot(Test.TestAnyPointer.factory);
MessageBuilder message2Builder =
new MessageBuilder(3 * 65535); // ample space to avoid far pointers
Test.TestAnyPointer.Builder message2RootBuilder =
message2Builder.getRoot(Test.TestAnyPointer.factory);
// Copy the struct that has the sentinel data.
message2RootBuilder.getAnyPointerField().setAs(Test.TestAnyPointer.factory, message1RootReader);
// Now clear the struct pointer.
message2RootBuilder.getAnyPointerField().clear();
java.nio.ByteBuffer[] outputSegments = message2Builder.getSegmentsForOutput();
Assert.assertEquals(1, outputSegments.length);
Assert.assertEquals(0L, outputSegments[0].getLong(8)); // null because cleared
Assert.assertEquals(16 + 8 * 65535, outputSegments[0].limit());
for (int ii = 0; ii < 65535; ++ii) {
// All of the data should have been cleared.
Assert.assertEquals(0L, outputSegments[0].getLong((2 + ii) * 8));
}
}
}

7
examples/pom.xml
View file

@ -5,7 +5,7 @@
<artifactId>examples</artifactId>
<packaging>jar</packaging>
<description>capnproto-java examples</description>
<version>0.1.10-SNAPSHOT</version>
<version>0.1.14-SNAPSHOT</version>
<name>capnproto-java examples</name>
<organization>
<name>org.capnproto</name>
@ -33,18 +33,19 @@
<project.build.sourceEncoding>UTF-8</project.build.sourceEncoding>
<maven.compiler.source>14</maven.compiler.source>
<maven.compiler.target>14</maven.compiler.target>
<maven.compiler.release>14</maven.compiler.release>
</properties>
<dependencies>
<dependency>
<groupId>org.capnproto</groupId>
<artifactId>runtime</artifactId>
<version>0.1.10-SNAPSHOT</version>
<version>0.1.14-SNAPSHOT</version>
</dependency>
<dependency>
<groupId>org.capnproto</groupId>
<artifactId>runtime-rpc</artifactId>
<version>0.1.10-SNAPSHOT</version>
<version>0.1.14-SNAPSHOT</version>
</dependency>
<dependency>

2
pom.xml
View file

@ -4,7 +4,7 @@
<groupId>org.capnproto</groupId>
<artifactId>capnproto-java</artifactId>
<packaging>pom</packaging>
<version>0.1.10-SNAPSHOT</version>
<version>0.1.14-SNAPSHOT</version>
<name>Cap'n Proto for Java</name>
<url>https://capnproto.org/</url>
<modules>

2
runtime/pom.xml
View file

@ -5,7 +5,7 @@
<artifactId>runtime</artifactId>
<packaging>jar</packaging>
<description>runtime</description>
<version>0.1.10-SNAPSHOT</version>
<version>0.1.14-SNAPSHOT</version>
<name>Cap'n Proto runtime library</name>
<organization>
<name>org.capnproto</name>

8
runtime/src/main/java/org/capnproto/SegmentReader.java
View file

@ -38,4 +38,12 @@ public class SegmentReader {
public final long get(int index) {
return buffer.getLong(index * Constants.BYTES_PER_WORD);
}
// Verify that the `size`-long (in words) range starting at word index
// `start` is within bounds.
public final boolean in_bounds(int start, int size) {
if (start < 0 || size < 0) return false;
long sizeInWords = size * Constants.BYTES_PER_WORD;
return (long) start + sizeInWords < (long) this.buffer.capacity();
}
}

9
runtime/src/main/java/org/capnproto/Serialize.java
View file

@ -84,6 +84,10 @@ public final class Serialize {
segment0Size = firstWord.getInt(4);
}
if (segment0Size < 0) {
throw new DecodeException("segment 0 has more than 2^31 words, which is unsupported");
}
long totalWords = segment0Size;
// in words
@ -94,6 +98,11 @@ public final class Serialize {
fillBuffer(moreSizesRaw, bc);
for (int ii = 0; ii < segmentCount - 1; ++ii) {
int size = moreSizesRaw.getInt(ii * 4);
if (size < 0) {
throw new DecodeException("segment " + (ii + 1) +
" has more than 2^31 words, which is unsupported");
}
moreSizes.add(size);
totalWords += size;
}

10
runtime/src/main/java/org/capnproto/StructPointer.java
View file

@ -24,17 +24,17 @@ package org.capnproto;
import java.nio.ByteBuffer;
final class StructPointer{
public static short dataSize(long ref) {
public static int dataSize(long ref) {
// in words.
return (short)(WirePointer.upper32Bits(ref) & 0xffff);
return WirePointer.upper32Bits(ref) & 0xffff;
}
public static short ptrCount(long ref) {
return (short)(WirePointer.upper32Bits(ref) >>> 16);
public static int ptrCount(long ref) {
return WirePointer.upper32Bits(ref) >>> 16;
}
public static int wordSize(long ref) {
return (int)dataSize(ref) + (int)ptrCount(ref);
return dataSize(ref) + ptrCount(ref);
}
public static void setFromStructSize(ByteBuffer buffer, int offset, StructSize size) {

46
runtime/src/main/java/org/capnproto/WireHelpers.java
View file

@ -38,6 +38,14 @@ final class WireHelpers {
return (int)((bits + 63) / ((long) Constants.BITS_PER_WORD));
}
// ByteBuffer already does bounds checking, but we still want
// to check bounds in some places to avoid cpu amplification attacks.
static boolean bounds_check(SegmentReader segment,
int start,
int size) {
return segment == null || segment.in_bounds(start, size);
}
static class AllocateResult {
public final int ptr;
public final int refOffset;
@ -434,8 +442,8 @@ final class WireHelpers {
}
FollowBuilderFarsResult resolved = followBuilderFars(ref, target, segment);
short oldDataSize = StructPointer.dataSize(resolved.ref);
short oldPointerCount = StructPointer.ptrCount(resolved.ref);
int oldDataSize = StructPointer.dataSize(resolved.ref);
int oldPointerCount = StructPointer.ptrCount(resolved.ref);
int oldPointerSection = resolved.ptr + oldDataSize;
if (oldDataSize < size.data || oldPointerCount < size.pointers) {
@ -482,7 +490,7 @@ final class WireHelpers {
} else {
return factory.constructBuilder(resolved.segment, capTable, resolved.ptr * Constants.BYTES_PER_WORD,
oldPointerSection, oldDataSize * Constants.BITS_PER_WORD,
oldPointerCount);
(short)oldPointerCount);
}
}
@ -623,7 +631,7 @@ final class WireHelpers {
throw new DecodeException("INLINE_COMPOSITE list with non-STRUCT elements not supported.");
}
int oldDataSize = StructPointer.dataSize(oldTag);
short oldPointerCount = StructPointer.ptrCount(oldTag);
int oldPointerCount = StructPointer.ptrCount(oldTag);
int oldStep = (oldDataSize + oldPointerCount * Constants.POINTER_SIZE_IN_WORDS);
int elementCount = WirePointer.inlineCompositeListElementCount(oldTag);
@ -632,7 +640,8 @@ final class WireHelpers {
return factory.constructBuilder(resolved.segment, capTable, oldPtr * Constants.BYTES_PER_WORD,
elementCount,
oldStep * Constants.BITS_PER_WORD,
oldDataSize * Constants.BITS_PER_WORD, oldPointerCount);
oldDataSize * Constants.BITS_PER_WORD,
(short)oldPointerCount);
}
//# The structs in this list are smaller than expected, probably written using an older
@ -951,7 +960,7 @@ final class WireHelpers {
resolved.ptr * Constants.BYTES_PER_WORD,
(resolved.ptr + dataSizeWords),
dataSizeWords * Constants.BITS_PER_WORD,
StructPointer.ptrCount(resolved.ref),
(short)StructPointer.ptrCount(resolved.ref),
nestingLimit - 1);
}
@ -993,7 +1002,7 @@ final class WireHelpers {
resolved.ptr * Constants.BYTES_PER_WORD,
(resolved.ptr + dataSizeWords),
dataSizeWords * Constants.BITS_PER_WORD,
StructPointer.ptrCount(resolved.ref),
(short)StructPointer.ptrCount(resolved.ref),
nestingLimit - 1);
}
@ -1004,7 +1013,7 @@ final class WireHelpers {
AllocateResult allocation = allocate(refOffset, segment, capTable, totalSize, WirePointer.STRUCT);
StructPointer.set(allocation.segment.buffer, allocation.refOffset,
dataSize, value.pointerCount);
(short)dataSize, value.pointerCount);
if (value.dataSize == 1) {
throw new RuntimeException("single bit case not handled");
@ -1131,7 +1140,7 @@ final class WireHelpers {
resolved.ptr * Constants.BYTES_PER_WORD,
resolved.ptr + StructPointer.dataSize(resolved.ref),
StructPointer.dataSize(resolved.ref) * Constants.BITS_PER_WORD,
StructPointer.ptrCount(resolved.ref),
(short)StructPointer.ptrCount(resolved.ref),
nestingLimit - 1));
case WirePointer.LIST :
byte elementSize = ListPointer.elementSize(resolved.ref);
@ -1167,7 +1176,7 @@ final class WireHelpers {
elementCount,
wordsPerElement * Constants.BITS_PER_WORD,
StructPointer.dataSize(tag) * Constants.BITS_PER_WORD,
StructPointer.ptrCount(tag),
(short)StructPointer.ptrCount(tag),
nestingLimit - 1));
} else {
int dataSize = ElementSize.dataBitsPerElement(elementSize);
@ -1238,6 +1247,10 @@ final class WireHelpers {
FollowFarsResult resolved = followFars(ref, refTarget, segment);
if (WirePointer.kind(resolved.ref) != WirePointer.LIST) {
throw new DecodeException("Message contains non-list pointer where list was expected.");
}
byte elementSize = ListPointer.elementSize(resolved.ref);
switch (elementSize) {
case ElementSize.INLINE_COMPOSITE : {
@ -1247,6 +1260,9 @@ final class WireHelpers {
int ptr = resolved.ptr + 1;
resolved.segment.arena.checkReadLimit(wordCount + 1);
if (!bounds_check(resolved.segment, resolved.ptr, wordCount + 1)) {
throw new DecodeException("Message contains out-of-bounds list pointer");
}
int size = WirePointer.inlineCompositeListElementCount(tag);
@ -1269,7 +1285,7 @@ final class WireHelpers {
size,
wordsPerElement * Constants.BITS_PER_WORD,
StructPointer.dataSize(tag) * Constants.BITS_PER_WORD,
StructPointer.ptrCount(tag),
(short)StructPointer.ptrCount(tag),
nestingLimit - 1);
}
default : {
@ -1282,8 +1298,12 @@ final class WireHelpers {
int elementCount = ListPointer.elementCount(resolved.ref);
int step = dataSize + pointerCount * Constants.BITS_PER_POINTER;
resolved.segment.arena.checkReadLimit(
roundBitsUpToWords(elementCount * step));
int wordCount = roundBitsUpToWords((long)elementCount * step);
resolved.segment.arena.checkReadLimit(wordCount);
if (!bounds_check(resolved.segment, resolved.ptr, wordCount)) {
throw new DecodeException("Message contains out-of-bounds list pointer");
}
if (elementSize == ElementSize.VOID) {
// Watch out for lists of void, which can claim to be arbitrarily large without

18
runtime/src/test/java/org/capnproto/SerializeTest.java
View file

@ -176,4 +176,22 @@ public class SerializeTest {
3, 0, 0, 0, 0, 0, 0, 0
});
}
@Test(expected=DecodeException.class)
public void testSegment0SizeOverflow() throws java.io.IOException {
byte[] input = {0, 0, 0, 0, -1, -1, -1, -113};
java.nio.channels.ReadableByteChannel channel =
java.nio.channels.Channels.newChannel(new java.io.ByteArrayInputStream(input));
MessageReader message = Serialize.read(channel);
}
@Test(expected=DecodeException.class)
public void testSegment1SizeOverflow() throws java.io.IOException {
byte[] input = {
1, 0, 0, 0, 1, 0, 0, 0,
-1, -1, -1, -113, 0, 0, 0, 0};
java.nio.channels.ReadableByteChannel channel =
java.nio.channels.Channels.newChannel(new java.io.ByteArrayInputStream(input));
MessageReader message = Serialize.read(channel);
}
}

48
runtime/src/test/java/org/capnproto/StructPointerTest.java Normal file
View file

@ -0,0 +1,48 @@
package org.capnproto;
import org.junit.Assert;
import org.junit.Test;
public class StructPointerTest {
@Test
public void testDataSize() {
Assert.assertEquals(
2,
StructPointer.dataSize(0x0001000200000000L));
}
@Test
public void testDataSizeUnderflow() {
Assert.assertEquals(
0xffff,
StructPointer.dataSize(0x0001ffff00000000L));
}
@Test
public void testPtrCount() {
Assert.assertEquals(
1,
StructPointer.ptrCount(0x0001000200000000L));
}
@Test
public void testPtrCountUnderflow() {
Assert.assertEquals(
0xffff,
StructPointer.ptrCount(0xffff000200000000L));
}
@Test
public void testWordSize() {
Assert.assertEquals(
3,
StructPointer.wordSize(0x0001000200000000L));
}
@Test
public void testWordSizeUnderflow() {
Assert.assertEquals(
0x1fffe,
StructPointer.wordSize(0xffffffff00000000L));
}
}

57
security-advisories/2021-09-30-0-cpu-amplification.md Normal file
View file

@ -0,0 +1,57 @@
Problem
=======
CPU usage amplification attack
Discovered by
=============
Martin Dindoffer &lt;contact@dindoffer.eu>
Announced
=========
2021-09-30
Impact
======
It is possible for an attacker to craft a small malicious message that will contain large lists with no data, leading to
artificial load on the system and possibly a Denial of Service attack.
CVSS score
==========
5.9 (Medium) CVSS:3.1/AV:N/AC:H/PR:N/UI:N/S:U/C:N/I:N/A:H
Fixed in
========
Release 0.1.11
Details
=======
Cap'n'Proto List pointer encodes the size of the list in the header as a 29 bit value. This value is read as a counter
for the list Iterator. Setting a large value for the elementCount may produce empty loops in the code that don't operate
on any data but still consume CPU time.
List Amplification is a well known posibility in the Cap'n'Proto protocol, as can be seen in the Encoding
spec (https://capnproto.org/encoding.html):
> A list of Void values or zero-size structs can have a very large element count while taking constant space on the wire.
> If the receiving application expects a list of structs, it will see these zero-sized elements as valid structs set to their default values.
> If it iterates through the list processing each element, it could spend a large amount of CPU time or other resources despite the message being small.
> To defend against this, the “traversal limit” should count a list of zero-sized elements as if each element were one word instead.
> This rule was introduced in the C++ implementation in commit 1048706.
A form of this traversal limit countermeasure is present in capnproto-java. However, the message may contain a huge list
with 1-bit elements that are read as a struct list. This combined with the fact that the data may simply be stripped
from the message makes it is possible to create problematically huge Iterators with default traversal limits.
Remediation
===========
The iteration should take into account the actual message length. As a rule of thumb the parser should not enter loops
with an attacker-controlled iteration count that is not bounded by the input length.
Therefore additional bounds checking was introduced to detect out-of-bounds list pointers.

54
security-advisories/2021-09-30-1-excessive-memory-allocation.md Normal file
View file

@ -0,0 +1,54 @@
Problem
=======
Insufficient validation of message metadata with negative segment sizes may lead to excessive memory allocation and
subsequent DoS.
Discovered by
=============
Martin Dindoffer &lt;contact@dindoffer.eu>
Announced
=========
2021-09-30
Impact
======
It is possible for an attacker to craft a malicious message that may thanks to memory amplification lead to a Denial of
Service attack against the consumer of CapnProto messages. In practical terms a message of only 8 bytes may cause
allocation of 2GB of memory.
CVSS score
==========
7.5 (High) CVSS:3.1/AV:N/AC:L/PR:N/UI:N/S:U/C:N/I:N/A:H
Fixed in
========
Release 0.1.11
Details
=======
A read of 4 bytes from the message is performed, the result of which is read as the size of a segment in a variable.
This size is stored in memory as a signed integer. If the segment count is 1, the size of the first segment is also
considered to be the total size of the message stored in a totalWords variable. Since both the segment0Size and the
totalWords are signed integers, they may hold a negative number which will pass the traversal limit check.
Subsequently capnproto-java tries to allocate enough memory for the segment. Because the encoded size is in words, the
value is multiplied by the number 8. The resulting number is used as the size of a ByteBuffer allocated for the first
segment. A sufficiently large negative number when multiplied by 8 will net a large positive integer, thus causing large
memory allocation in the form of a ByteBuffer.
A segment size of `-1 879 048 193` multiplied by 8 results in `2 147 483 640` bytes of allocated memory, which is close
to `Integer.MAX_VALUE` and around the size of the maximum length of a Java array. Therefore the maximum size a message
may allocate this way is roughly 2GB.
Remediation
===========
Stricter validation refusing negative segment sizes was implemented.

47
security-advisories/2021-10-04-0-cpu-amplification.md Normal file
View file

@ -0,0 +1,47 @@
Problem
=======
Incorrect parsing of large list element sizes may lead to CPU usage amplification attack
Discovered by
=============
Martin Dindoffer &lt;contact@dindoffer.eu>
Announced
=========
2021-10-04
Impact
======
It is possible for an attacker to craft a small malicious message that will contain large lists with no data, leading to
artificial load on the system and possibly a Denial of Service attack.
CVSS score
==========
5.9 (Medium) CVSS:3.1/AV:N/AC:H/PR:N/UI:N/S:U/C:N/I:N/A:H
Fixed in
========
Release 0.1.12
Details
=======
A tag pointer encodes the size of the elements in words and the total number of elements. Validation of these values is
performed in the code, but only as an overflow check of their product (against the total wordcount encoded in the list
pointer). The element size is read as a `short` value with improper casting to signed integers. Therefore, it is
possible to craft a message that will encode a large amount of elements (up to `2^30`) and offset this bloated size by
specifying a negative size of the elements (in words). There is no need to encode the actual payload for the large list,
thus capnproto-java will generate Iterators with large iteration counts over empty payload with messages of well under
100 bytes, that may put CPU under loads heavy enough to easily perform DoS attacks.
Remediation
===========
The `short` values of the list element sizes are now parsed as unsigned values, making sure that the list overflow check
will catch the huge list size.