Repository: amzn/ion-java
Pull request #161 (2018-10-05 01:11:13)

Bound checking BigDecimal in Timestamp to guard against DoS
Operations that require inflating the BigDecimal can be expensive for
large numbers, examples:
* longValue
* intValue
* setScale

https://github.com/amzn/ion-java/issues/159


By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.



Patch:
diff --git a/src/software/amazon/ion/Timestamp.java b/src/software/amazon/ion/Timestamp.java
index 89b14f0b..1d668bb9 100644
--- a/src/software/amazon/ion/Timestamp.java
+++ b/src/software/amazon/ion/Timestamp.java
@@ -84,6 +84,15 @@ public final class Timestamp
     private static final int NO_SECONDS = 0;
     private static final BigDecimal NO_FRACTIONAL_SECONDS = null;
 
+    // 0001-01-01T00:00:00.0Z in millis
+    private static final long MINIMUM_ALLOWED_TIMESTAMP_IN_MILLIS = -62135769600000L;
+
+    // 0001-01-01T00:00:00.0Z in millis
+    static final BigDecimal MINIMUM_ALLOWED_FRACTIONAL_MILLIS = new BigDecimal(MINIMUM_ALLOWED_TIMESTAMP_IN_MILLIS);
+
+    // 10000T in millis, upper bound exclusive
+    static final BigDecimal FRACTIONAL_MILLIS_UPPER_BOUND = new BigDecimal(253402300800000L);
+
     /**
      * Unknown local offset from UTC.
      */
@@ -266,9 +275,6 @@ public final class Timestamp
         }
     }
 
-    // 0001-01-01T00:00:00.0Z in millis
-    private static final long MINIMUM_ALLOWED_TIMESTAMP_IN_MILLIS = -62135769600000L;
-
     /**
      * This method uses deprecated methods from {@link java.util.Date}
      * instead of {@link Calendar} so that this code can be used (more easily)
@@ -704,7 +710,22 @@ public final class Timestamp
     {
         if (millis == null) throw new NullPointerException("millis is null");
 
-        long ms = millis.longValue();
+        // check bounds to avoid hanging when calling longValue() on decimals with large positive exponents,
+        // e.g. 1e10000000
+        if(millis.compareTo(MINIMUM_ALLOWED_FRACTIONAL_MILLIS) < 0 ||
+            FRACTIONAL_MILLIS_UPPER_BOUND.compareTo(millis) < 0) {
+            throw new IllegalArgumentException("millis: " + millis + " is outside of valid the range: from "
+                + MINIMUM_ALLOWED_FRACTIONAL_MILLIS
+                + " (0001T)"
+                + ", inclusive, to "
+                + FRACTIONAL_MILLIS_UPPER_BOUND
+                + " (10000T)"
+                + " , exclusive");
+        }
+
+        // quick handle integral zero
+        long ms = isIntegralZero(millis) ? 0 : millis.longValue();
+
         set_fields_from_millis(ms);
 
         this._precision = Precision.SECOND;
@@ -714,12 +735,19 @@ public final class Timestamp
         }
         else {
             BigDecimal secs = millis.movePointLeft(3);
-            BigDecimal secsDown = secs.setScale(0, RoundingMode.FLOOR);
+            BigDecimal secsDown = isIntegralZero(secs) ? BigDecimal.ZERO : secs.setScale(0, BigDecimal.ROUND_FLOOR);
             this._fraction = secs.subtract(secsDown);
         }
         this._offset = localOffset;
     }
 
+    private boolean isIntegralZero(final BigDecimal decimal) {
+        // zero || no low-order bits || < 1.0
+        return  decimal.signum() == 0
+            || decimal.scale() < -63
+            || (decimal.precision() - decimal.scale() <= 0);
+    }
+
     /**
      * Creates a new Timestamp that represents the point in time that is
      * {@code millis} milliseconds from the epoch, with a given local offset.
@@ -1418,7 +1446,8 @@ public final class Timestamp
         //                                        month is 0 based for Date
         long millis = Date.UTC(this._year - 1900, this._month - 1, this._day, this._hour, this._minute, this._second);
         if (this._fraction != null) {
-            int frac = this._fraction.movePointRight(3).intValue();
+            BigDecimal fracAsDecimal = this._fraction.movePointRight(3);
+            int frac = isIntegralZero(fracAsDecimal) ? 0 : fracAsDecimal.intValue();
             millis += frac;
         }
         return millis;
@@ -1816,7 +1845,6 @@ public final class Timestamp
         return buffer.toString();
     }
 
-
     /**
      * Returns the string representation (in Ion format) of this Timestamp
      * in UTC.
@@ -1834,12 +1862,11 @@ public final class Timestamp
         catch (IOException e)
         {
             throw new RuntimeException("Exception printing to StringBuilder",
-                                       e);
+                e);
         }
         return buffer.toString();
     }
 
-
     /**
      * Prints to an {@code Appendable} the string representation (in Ion format)
      * of this Timestamp in its local time.
@@ -1868,7 +1895,6 @@ public final class Timestamp
         print(out, adjusted);
     }
 
-
     /**
      * Prints to an {@code Appendable} the string representation (in Ion format)
      * of this Timestamp in UTC.
@@ -1906,7 +1932,6 @@ public final class Timestamp
         }
     }
 
-
     /**
      * helper for print(out) and printZ(out) so that printZ can create
      * a zulu time and pass it directly and print can apply the local


Test patch:
diff --git a/test/software/amazon/ion/TimestampTest.java b/test/software/amazon/ion/TimestampTest.java
index c0017c75..28bffb87 100644
--- a/test/software/amazon/ion/TimestampTest.java
+++ b/test/software/amazon/ion/TimestampTest.java
@@ -16,6 +16,8 @@ package software.amazon.ion;
 
 import static software.amazon.ion.Decimal.NEGATIVE_ZERO;
 import static software.amazon.ion.Decimal.negativeZero;
+import static software.amazon.ion.Timestamp.FRACTIONAL_MILLIS_UPPER_BOUND;
+import static software.amazon.ion.Timestamp.MINIMUM_ALLOWED_FRACTIONAL_MILLIS;
 import static software.amazon.ion.Timestamp.UNKNOWN_OFFSET;
 import static software.amazon.ion.Timestamp.UTC_OFFSET;
 import static software.amazon.ion.Timestamp.createFromUtcFields;
@@ -26,19 +28,17 @@ import static software.amazon.ion.Timestamp.Precision.SECOND;
 import static software.amazon.ion.Timestamp.Precision.YEAR;
 import static software.amazon.ion.impl.PrivateUtils.UTC;
 
+import java.io.IOException;
 import java.math.BigDecimal;
+import java.sql.Time;
 import java.text.DateFormat;
 import java.text.SimpleDateFormat;
+import java.time.Instant;
 import java.util.Calendar;
 import java.util.Date;
 import java.util.TimeZone;
+import org.junit.Ignore;
 import org.junit.Test;
-import software.amazon.ion.Decimal;
-import software.amazon.ion.IonTimestamp;
-import software.amazon.ion.IonType;
-import software.amazon.ion.IonValue;
-import software.amazon.ion.NullValueException;
-import software.amazon.ion.Timestamp;
 import software.amazon.ion.Timestamp.Precision;
 
 /**
@@ -781,12 +781,47 @@ public class TimestampTest
         assertEquals("2012-01-01T12:12:30.555123Z", ts.toZString());
     }
 
+    @Test
+    @Ignore // see https://github.com/amzn/ion-java/issues/160
+    public void testNewTimestampFromMinimumAllowedMillis()
+    {
+        Timestamp ts = Timestamp.forMillis(MINIMUM_ALLOWED_FRACTIONAL_MILLIS, PST_OFFSET);
+        assertEquals("0001-01-01T00:00:00.000Z", ts.toZString());
+    }
+
+    @Test
+    public void testNewTimestampFromBigDecimalWithMaximumAllowedMillis()
+    {
+        Timestamp ts = Timestamp.forMillis(FRACTIONAL_MILLIS_UPPER_BOUND.add(BigDecimal.ONE.negate()), PST_OFFSET);
+        checkFields(9999, 12, 31, 15, 59, 59, new BigDecimal("0.999"), PST_OFFSET, SECOND, ts);
+        assertEquals("9999-12-31T15:59:59.999-08:00", ts.toString());
+        assertEquals("9999-12-31T23:59:59.999Z", ts.toZString());
+    }
 
-    @SuppressWarnings("unused")
-    @Test (expected = NullPointerException.class)
+    @Test(expected = NullPointerException.class)
     public void testNewTimestampFromBigDecimalWithNull()
     {
-        Timestamp ts = Timestamp.forMillis(null, PST_OFFSET);
+        Timestamp.forMillis(null, PST_OFFSET);
+    }
+
+    @Test(expected = IllegalArgumentException.class)
+    public void testNewTimestampFromBigDecimalWithMillisTooSmall()
+    {
+        // MIN - 1
+        Timestamp.forMillis(MINIMUM_ALLOWED_FRACTIONAL_MILLIS.add(BigDecimal.ONE.negate()), PST_OFFSET);
+    }
+
+    @Test(expected = IllegalArgumentException.class)
+    public void testNewTimestampFromBigDecimalWithMillisTooBig()
+    {
+        // Max
+        Timestamp.forMillis(FRACTIONAL_MILLIS_UPPER_BOUND, PST_OFFSET);
+    }
+
+    @Test(expected = IllegalArgumentException.class)
+    public void testNewTimestampFromBigDecimalWithScaleTooBigPositive()
+    {
+        Timestamp.forMillis(new BigDecimal("1e100000"), PST_OFFSET);
     }
 
     /**
@@ -2307,4 +2342,21 @@ public class TimestampTest
         Timestamp t = Timestamp.forCalendar(cal);
         assertEquals(year, t.getYear());
     }
+
+    // High scale timeout tests
+
+    // max scale permitted by BigDecimal from the String constructor
+    private static BigDecimal LARGE_SCALE_DECIMAL = new BigDecimal("1e-1000000000");
+
+    @Test(timeout = 50L)
+    public void testForMillisWithLargeScaleBigDecimal()
+    {
+        Timestamp ts = Timestamp.forMillis(LARGE_SCALE_DECIMAL, PST_OFFSET);
+    }
+
+    @Test(timeout = 50L)
+    public void testGetMillisWithLargeScaleBigDecimal()
+    {
+        Timestamp.forMillis(LARGE_SCALE_DECIMAL, PST_OFFSET).getMillis();
+    }
 }