This is an automated email from the ASF dual-hosted git repository.
jmckenzie pushed a commit to branch cassandra-3.11
in repository https://gitbox.apache.org/repos/asf/cassandra.git
The following commit(s) were added to refs/heads/cassandra-3.11 by this push:
new ab481be81d Fix potential IndexOutOfBoundsException in PagingState in
mixed mode clusters
ab481be81d is described below
commit ab481be81da0f06a80e099ca7502b7453ff568d3
Author: Josh McKenzie <jmcken...@apache.org>
AuthorDate: Tue Aug 23 14:37:17 2022 -0400
Fix potential IndexOutOfBoundsException in PagingState in mixed mode
clusters
Patch by Alex Petrov; reviewed by Josh McKenzie, Sam Tunnicliffe, and
Aleksey Yeschenko for CASSANDRA-17840
Co-authored-by: Alex Petrov <oleksandr.pet...@gmail.com>
Co-authored-by: Josh McKenzie <jmcken...@apache.org>
---
CHANGES.txt | 1 +
.../org/apache/cassandra/db/BufferClustering.java | 5 +-
.../cassandra/service/pager/PagingState.java | 54 ++++++++---
.../apache/cassandra/utils/vint/VIntCoding.java | 3 +
.../service/pager/RandomizedPagingStateTest.java | 103 +++++++++++++++++++++
5 files changed, 153 insertions(+), 13 deletions(-)
diff --git a/CHANGES.txt b/CHANGES.txt
index bacd5b0767..c9f3b7630a 100644
--- a/CHANGES.txt
+++ b/CHANGES.txt
@@ -1,4 +1,5 @@
3.11.14
+ * Fix potential IndexOutOfBoundsException in PagingState in mixed mode
clusters (CASSANDRA-17840)
* Document usage of closed token intervals in manual compaction
(CASSANDRA-17575)
* Creating of a keyspace on insufficient number of replicas should filter out
gosspping-only members (CASSANDRA-17759)
* Only use statically defined subcolumns when determining column definition
for supercolumn cell (CASSANDRA-14113)
diff --git a/src/java/org/apache/cassandra/db/BufferClustering.java
b/src/java/org/apache/cassandra/db/BufferClustering.java
index 7ca91320e8..8cb8a59902 100644
--- a/src/java/org/apache/cassandra/db/BufferClustering.java
+++ b/src/java/org/apache/cassandra/db/BufferClustering.java
@@ -19,6 +19,8 @@ package org.apache.cassandra.db;
import java.nio.ByteBuffer;
+import com.google.common.annotations.VisibleForTesting;
+
import org.apache.cassandra.utils.ByteBufferUtil;
/**
@@ -35,7 +37,8 @@ import org.apache.cassandra.utils.ByteBufferUtil;
*/
public class BufferClustering extends AbstractBufferClusteringPrefix
implements Clustering
{
- BufferClustering(ByteBuffer... values)
+ @VisibleForTesting
+ public BufferClustering(ByteBuffer... values)
{
super(Kind.CLUSTERING, values);
}
diff --git a/src/java/org/apache/cassandra/service/pager/PagingState.java
b/src/java/org/apache/cassandra/service/pager/PagingState.java
index 9b7eccf321..10044a822d 100644
--- a/src/java/org/apache/cassandra/service/pager/PagingState.java
+++ b/src/java/org/apache/cassandra/service/pager/PagingState.java
@@ -134,39 +134,68 @@ public class PagingState
return out.buffer(false);
}
- private static boolean isModernSerialized(ByteBuffer bytes)
+ @VisibleForTesting
+ static boolean isModernSerialized(ByteBuffer bytes)
{
int index = bytes.position();
int limit = bytes.limit();
- long partitionKeyLen = getUnsignedVInt(bytes, index, limit);
+ int partitionKeyLen = toIntExact(getUnsignedVInt(bytes, index, limit));
if (partitionKeyLen < 0)
return false;
- index += computeUnsignedVIntSize(partitionKeyLen) + partitionKeyLen;
- if (index >= limit)
+ index = addNonNegative(index,
computeUnsignedVIntSize(partitionKeyLen), partitionKeyLen);
+ if (index >= limit || index < 0)
return false;
- long rowMarkerLen = getUnsignedVInt(bytes, index, limit);
+ int rowMarkerLen = toIntExact(getUnsignedVInt(bytes, index, limit));
if (rowMarkerLen < 0)
return false;
- index += computeUnsignedVIntSize(rowMarkerLen) + rowMarkerLen;
- if (index >= limit)
+ index = addNonNegative(index, computeUnsignedVIntSize(rowMarkerLen),
rowMarkerLen);
+ if (index >= limit || index < 0)
return false;
- long remaining = getUnsignedVInt(bytes, index, limit);
+ int remaining = toIntExact(getUnsignedVInt(bytes, index, limit));
if (remaining < 0)
return false;
- index += computeUnsignedVIntSize(remaining);
- if (index >= limit)
+ index = addNonNegative(index, computeUnsignedVIntSize(remaining));
+ if (index >= limit || index < 0)
return false;
long remainingInPartition = getUnsignedVInt(bytes, index, limit);
if (remainingInPartition < 0)
return false;
- index += computeUnsignedVIntSize(remainingInPartition);
+ index = addNonNegative(index,
computeUnsignedVIntSize(remainingInPartition));
return index == limit;
}
+ // Following operations are similar to Math.{addExact/toIntExact}, but
without using exceptions for control flow.
+ // Since we're operating non-negative numbers, we can use -1 return value
as an error code.
+ private static int addNonNegative(int x, int y)
+ {
+ int sum = x + y;
+ if (sum < 0)
+ return -1;
+ return sum;
+ }
+
+ private static int addNonNegative(int x, int y, int z)
+ {
+ int sum = x + y;
+ if (sum < 0)
+ return -1;
+ sum += z;
+ if (sum < 0)
+ return -1;
+ return sum;
+ }
+
+ private static int toIntExact(long value)
+ {
+ if ((int)value != value)
+ return -1;
+ return (int)value;
+ }
+
@SuppressWarnings({ "resource", "RedundantSuppression" })
private static PagingState modernDeserialize(ByteBuffer bytes,
ProtocolVersion protocolVersion) throws IOException
{
@@ -214,7 +243,8 @@ public class PagingState
return out.buffer(false);
}
- private static boolean isLegacySerialized(ByteBuffer bytes)
+ @VisibleForTesting
+ static boolean isLegacySerialized(ByteBuffer bytes)
{
int index = bytes.position();
int limit = bytes.limit();
diff --git a/src/java/org/apache/cassandra/utils/vint/VIntCoding.java
b/src/java/org/apache/cassandra/utils/vint/VIntCoding.java
index ac5a74e2e0..68ebf42aea 100644
--- a/src/java/org/apache/cassandra/utils/vint/VIntCoding.java
+++ b/src/java/org/apache/cassandra/utils/vint/VIntCoding.java
@@ -97,6 +97,9 @@ public class VIntCoding
public static long getUnsignedVInt(ByteBuffer input, int readerIndex, int
readerLimit)
{
+ if (readerIndex < 0)
+ throw new IllegalArgumentException("Reader index should be
non-negative, but was " + readerIndex);
+
if (readerIndex >= readerLimit)
return -1;
diff --git
a/test/unit/org/apache/cassandra/service/pager/RandomizedPagingStateTest.java
b/test/unit/org/apache/cassandra/service/pager/RandomizedPagingStateTest.java
new file mode 100644
index 0000000000..4c9592a25d
--- /dev/null
+++
b/test/unit/org/apache/cassandra/service/pager/RandomizedPagingStateTest.java
@@ -0,0 +1,103 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.cassandra.service.pager;
+
+import java.nio.ByteBuffer;
+import java.util.Random;
+
+import org.junit.Assert;
+import org.junit.Test;
+
+import org.apache.cassandra.config.CFMetaData;
+import org.apache.cassandra.config.DatabaseDescriptor;
+import org.apache.cassandra.db.BufferClustering;
+import org.apache.cassandra.db.Clustering;
+import org.apache.cassandra.db.marshal.BytesType;
+import org.apache.cassandra.db.marshal.LongType;
+import org.apache.cassandra.db.rows.BTreeRow;
+import org.apache.cassandra.db.rows.BufferCell;
+import org.apache.cassandra.db.rows.Row;
+import org.apache.cassandra.transport.ProtocolVersion;
+import org.apache.cassandra.utils.ByteBufferUtil;
+
+public class RandomizedPagingStateTest
+{
+ static
+ {
+ DatabaseDescriptor.daemonInitialization();
+ }
+
+ private static final Random rnd = new Random();
+ private static final int ROUNDS = 50_000;
+ private static final int MAX_PK_SIZE = 3000;
+ private static final int MAX_CK_SIZE = 3000;
+ private static final int MAX_REMAINING = 5000;
+
+ @Test
+ public void testFormatChecksPkOnly()
+ {
+ rnd.setSeed(1);
+ CFMetaData metadata = CFMetaData.Builder.create("ks", "tbl")
+ .addPartitionKey("pk",
BytesType.instance)
+ .addRegularColumn("v",
LongType.instance)
+ .build();
+ Row row = BTreeRow.emptyRow(Clustering.EMPTY);
+ for (int i = 0; i < ROUNDS; i++)
+ checkState(metadata, MAX_PK_SIZE, row);
+ }
+
+ @Test
+ public void testFormatChecksPkAndCk()
+ {
+ rnd.setSeed(1);
+ CFMetaData metadata = CFMetaData.Builder.create("ks", "tbl")
+ .addPartitionKey("pk",
BytesType.instance)
+ .addClusteringColumn("ck",
BytesType.instance)
+ .addRegularColumn("v",
LongType.instance)
+ .build();
+ for (int i = 0; i < ROUNDS; i++)
+ {
+ ByteBuffer ckBytes = ByteBuffer.allocate(rnd.nextInt(MAX_CK_SIZE)
+ 1);
+ for (int j = 0; j < ckBytes.limit(); j++)
+ ckBytes.put((byte) rnd.nextInt());
+ ckBytes.flip().rewind();
+
+ Clustering c = new BufferClustering(ckBytes);
+ Row row = BTreeRow.singleCellRow(c,
+
BufferCell.live(metadata.partitionColumns().iterator().next(),
+ 0,
+
ByteBufferUtil.EMPTY_BYTE_BUFFER));
+
+ checkState(metadata, 1, row);
+ }
+ }
+
+ private static void checkState(CFMetaData metadata, int maxPkSize, Row row)
+ {
+ PagingState.RowMark mark = PagingState.RowMark.create(metadata, row,
ProtocolVersion.V3);
+ ByteBuffer pkBytes = ByteBuffer.allocate(rnd.nextInt(maxPkSize) + 1);
+ for (int j = 0; j < pkBytes.limit(); j++)
+ pkBytes.put((byte) rnd.nextInt());
+ pkBytes.flip().rewind();
+
+ PagingState state = new PagingState(pkBytes, mark,
rnd.nextInt(MAX_REMAINING) + 1, rnd.nextInt(MAX_REMAINING) + 1);
+ ByteBuffer serialized = state.serialize(ProtocolVersion.V3);
+ Assert.assertTrue(PagingState.isLegacySerialized(serialized));
+ Assert.assertFalse(PagingState.isModernSerialized(serialized));
+ }
+}
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org
For additional commands, e-mail: commits-h...@cassandra.apache.org
