Fix duration type validation to prevent overflow

patch by Benjamin Lerer; reviewed by Andrés de la Peña for CASSANDRA-13218


Project: http://git-wip-us.apache.org/repos/asf/cassandra/repo
Commit: http://git-wip-us.apache.org/repos/asf/cassandra/commit/86933571
Tree: http://git-wip-us.apache.org/repos/asf/cassandra/tree/86933571
Diff: http://git-wip-us.apache.org/repos/asf/cassandra/diff/86933571

Branch: refs/heads/trunk
Commit: 8693357109a6e59117a641e109c3865501e3eee6
Parents: f33cdcb
Author: Benjamin Lerer <b.le...@gmail.com>
Authored: Thu May 11 10:26:36 2017 +0200
Committer: Benjamin Lerer <b.le...@gmail.com>
Committed: Thu May 11 10:26:36 2017 +0200

----------------------------------------------------------------------
 CHANGES.txt                                     |  1 +
 doc/native_protocol_v5.spec                     |  2 ++
 doc/source/cql/types.rst                        |  2 ++
 .../cassandra/db/marshal/DurationType.java      |  1 -
 .../serializers/DurationSerializer.java         | 25 ++++++++++++++++++--
 .../cql3/validation/operations/CreateTest.java  |  8 ++++++-
 6 files changed, 35 insertions(+), 4 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/cassandra/blob/86933571/CHANGES.txt
----------------------------------------------------------------------
diff --git a/CHANGES.txt b/CHANGES.txt
index b8146af..bb13fc4 100644
--- a/CHANGES.txt
+++ b/CHANGES.txt
@@ -1,4 +1,5 @@
 3.11.0
+ * Fix duration type validation to prevent overflow (CASSANDRA-13218)
  * Forbid unsupported creation of SASI indexes over partition key columns 
(CASSANDRA-13228)
  * Reject multiple values for a key in CQL grammar. (CASSANDRA-13369)
  * UDA fails without input rows (CASSANDRA-13399)

http://git-wip-us.apache.org/repos/asf/cassandra/blob/86933571/doc/native_protocol_v5.spec
----------------------------------------------------------------------
diff --git a/doc/native_protocol_v5.spec b/doc/native_protocol_v5.spec
index ac3373c..36ffc4c 100644
--- a/doc/native_protocol_v5.spec
+++ b/doc/native_protocol_v5.spec
@@ -905,6 +905,8 @@ Table of Contents
   A duration is composed of 3 signed variable length integers ([vint]s).
   The first [vint] represents a number of months, the second [vint] represents
   a number of days, and the last [vint] represents a number of nanoseconds.
+  The number of months and days must be valid 32 bits integers whereas the
+  number of nanoseconds must be a valid 64 bits integer.
   A duration can either be positive or negative. If a duration is positive
   all the integers must be positive or zero. If a duration is
   negative all the numbers must be negative or zero.

http://git-wip-us.apache.org/repos/asf/cassandra/blob/86933571/doc/source/cql/types.rst
----------------------------------------------------------------------
diff --git a/doc/source/cql/types.rst b/doc/source/cql/types.rst
index 8954d87..509a756 100644
--- a/doc/source/cql/types.rst
+++ b/doc/source/cql/types.rst
@@ -189,6 +189,8 @@ Working with durations
 Values of the ``duration`` type are encoded as 3 signed integer of variable 
lengths. The first integer represents the
 number of months, the second the number of days and the third the number of 
nanoseconds. This is due to the fact that
 the number of days in a month can change, and a day can have 23 or 25 hours 
depending on the daylight saving.
+Internally, the number of months and days are decoded as 32 bits integers 
whereas the number of nanoseconds is decoded
+as a 64 bits integer.
 
 A duration can be input as:
 

http://git-wip-us.apache.org/repos/asf/cassandra/blob/86933571/src/java/org/apache/cassandra/db/marshal/DurationType.java
----------------------------------------------------------------------
diff --git a/src/java/org/apache/cassandra/db/marshal/DurationType.java 
b/src/java/org/apache/cassandra/db/marshal/DurationType.java
index 63e634c..b6f2062 100644
--- a/src/java/org/apache/cassandra/db/marshal/DurationType.java
+++ b/src/java/org/apache/cassandra/db/marshal/DurationType.java
@@ -28,7 +28,6 @@ import org.apache.cassandra.serializers.MarshalException;
 import org.apache.cassandra.serializers.TypeSerializer;
 import org.apache.cassandra.transport.ProtocolVersion;
 import org.apache.cassandra.utils.ByteBufferUtil;
-import org.apache.cassandra.utils.FastByteOperations;
 
 /**
  * Represents a duration. The duration is stored as  months, days, and 
nanoseconds. This is done

http://git-wip-us.apache.org/repos/asf/cassandra/blob/86933571/src/java/org/apache/cassandra/serializers/DurationSerializer.java
----------------------------------------------------------------------
diff --git a/src/java/org/apache/cassandra/serializers/DurationSerializer.java 
b/src/java/org/apache/cassandra/serializers/DurationSerializer.java
index 03d08ae..c752c40 100644
--- a/src/java/org/apache/cassandra/serializers/DurationSerializer.java
+++ b/src/java/org/apache/cassandra/serializers/DurationSerializer.java
@@ -83,10 +83,19 @@ public final class DurationSerializer implements 
TypeSerializer<Duration>
 
         try (DataInputBuffer in = new DataInputBuffer(bytes, true))
         {
-            int months = (int) in.readVInt();
-            int days = (int) in.readVInt();
+            long monthsAsLong = in.readVInt();
+            long daysAsLong = in.readVInt();
             long nanoseconds = in.readVInt();
 
+            if (!canBeCastToInt(monthsAsLong))
+                throw new MarshalException(String.format("The duration months 
must be a 32 bits integer but was: %d",
+                                                         monthsAsLong));
+            if (!canBeCastToInt(daysAsLong))
+                throw new MarshalException(String.format("The duration days 
must be a 32 bits integer but was: %d",
+                                                         daysAsLong));
+            int months = (int) monthsAsLong;
+            int days = (int) daysAsLong;
+
             if (!((months >= 0 && days >= 0 && nanoseconds >= 0) || (months <= 
0 && days <=0 && nanoseconds <=0)))
                 throw new MarshalException(String.format("The duration months, 
days and nanoseconds must be all of the same sign (%d, %d, %d)",
                                                          months, days, 
nanoseconds));
@@ -98,6 +107,18 @@ public final class DurationSerializer implements 
TypeSerializer<Duration>
         }
     }
 
+    /**
+     * Checks that the specified {@code long} can be cast to an {@code int} 
without information lost.
+     *
+     * @param l the {@code long} to check
+     * @return {@code true} if the specified {@code long} can be cast to an 
{@code int} without information lost,
+     * {@code false} otherwise.
+     */
+    private boolean canBeCastToInt(long l)
+    {
+        return ((int) l) == l;
+    }
+
     public String toString(Duration duration)
     {
         return duration == null ? "" : duration.toString();

http://git-wip-us.apache.org/repos/asf/cassandra/blob/86933571/test/unit/org/apache/cassandra/cql3/validation/operations/CreateTest.java
----------------------------------------------------------------------
diff --git 
a/test/unit/org/apache/cassandra/cql3/validation/operations/CreateTest.java 
b/test/unit/org/apache/cassandra/cql3/validation/operations/CreateTest.java
index 3eaec23..66078c5 100644
--- a/test/unit/org/apache/cassandra/cql3/validation/operations/CreateTest.java
+++ b/test/unit/org/apache/cassandra/cql3/validation/operations/CreateTest.java
@@ -170,6 +170,12 @@ public class CreateTest extends CQLTester
         assertInvalidMessage("The duration months, days and nanoseconds must 
be all of the same sign (-2, 0, 2000000)",
                              "INSERT INTO %s (a, b, c) VALUES (?, ?, ?)", 2, 
1, duration(-2, 0, 2000000));
 
+        assertInvalidMessage("The duration months must be a 32 bits integer 
but was: 9223372036854775807",
+                             "INSERT INTO %s (a, b, c) VALUES (?, ?, ?)", 2, 
1, duration(9223372036854775807L, 1, 0));
+
+        assertInvalidMessage("The duration days must be a 32 bits integer but 
was: 9223372036854775807",
+                             "INSERT INTO %s (a, b, c) VALUES (?, ?, ?)", 2, 
1, duration(0, 9223372036854775807L, 0));
+
         // Test with duration column name
         createTable("CREATE TABLE %s (a text PRIMARY KEY, duration 
duration);");
 
@@ -229,7 +235,7 @@ public class CreateTest extends CQLTester
                 "CREATE TABLE %s(pk int, m frozen<map<text, list<tuple<int, 
duration>>>>, v int, PRIMARY KEY (pk, m))");
     }
 
-    private ByteBuffer duration(int months, int days, long nanoseconds) throws 
IOException
+    private ByteBuffer duration(long months, long days, long nanoseconds) 
throws IOException
     {
         try(DataOutputBuffer output = new DataOutputBuffer())
         {


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org
For additional commands, e-mail: commits-h...@cassandra.apache.org

Reply via email to