Skip to content

Commit

Permalink
Bound checking BigDecimal in Timestamp to guard against DoS
Browse files Browse the repository at this point in the history
Operations that require inflating the BigDecimal can be expensive for
large numbers, examples:
* longValue
* intValue
* setScale

#159
  • Loading branch information
raganhan committed Oct 5, 2018
1 parent 88d5d45 commit c3d4828
Show file tree
Hide file tree
Showing 2 changed files with 81 additions and 12 deletions.
37 changes: 34 additions & 3 deletions src/software/amazon/ion/Timestamp.java
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,18 @@ 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);

// 9999-12-31T23:59:59.999999999999Z in millis
static final BigDecimal MAXIMUM_ALLOWED_FRACTIONAL_MILLIS = new BigDecimal("253402300799999.999999999");

// determined empirically as a safe scale
private static final int MAXIMUM_ALLOWED_MILLIS_SCALE = 100000;

/**
* Unknown local offset from UTC.
*/
Expand Down Expand Up @@ -265,9 +277,6 @@ private void apply_offset(int offset)
}
}

// 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)
Expand Down Expand Up @@ -690,6 +699,28 @@ private Timestamp(BigDecimal millis, Integer localOffset)
{
if (millis == null) throw new NullPointerException("millis is null");

// 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 ||
millis.compareTo(MAXIMUM_ALLOWED_FRACTIONAL_MILLIS) > 0) {
throw new IllegalArgumentException("millis: " + millis + " is outside of valid the range: from "
+ MINIMUM_ALLOWED_FRACTIONAL_MILLIS
+ " to "
+ MAXIMUM_ALLOWED_FRACTIONAL_MILLIS
+ " both inclusive");
}

// check scale size to avoid hanging on methods that need to inflate the fractional bigDecimal
// Examples: toString and toMillis
if(Math.abs(millis.scale()) >= MAXIMUM_ALLOWED_MILLIS_SCALE) {
throw new IllegalArgumentException("millis: " + millis + " has a scale outside the valid range:"
+ "from"
+ -MAXIMUM_ALLOWED_MILLIS_SCALE
+ "to"
+ MAXIMUM_ALLOWED_MILLIS_SCALE
+ "both inclusive");
}

long ms = millis.longValue();
set_fields_from_millis(ms);

Expand Down
56 changes: 47 additions & 9 deletions test/software/amazon/ion/TimestampTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@

import static software.amazon.ion.Decimal.NEGATIVE_ZERO;
import static software.amazon.ion.Decimal.negativeZero;
import static software.amazon.ion.Timestamp.MAXIMUM_ALLOWED_FRACTIONAL_MILLIS;
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;
Expand All @@ -32,13 +34,8 @@
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;

/**
Expand Down Expand Up @@ -781,12 +778,53 @@ public void testNewTimestampFromBigDecimal()
assertEquals("2012-01-01T12:12:30.555123Z", ts.toZString());
}

@Test
@Ignore // see https://github.com/amzn/ion-java/issues/160
public void testNewTimestampFromMinimumAllowedMillis()

This comment has been minimized.

Copy link
@Bacha0512

Bacha0512 Oct 5, 2018

System.out.println(t2); // 0002-01-01T00:00:00.000Z
System.out.println(t2.getMillis()); // -62104233600000

This comment has been minimized.

Copy link
@raganhan

raganhan Oct 5, 2018

Author Contributor

not sure what you mean by your comment. This test is ignore because of the issue #160.

the minimum allowed timestamp is 0001-01-01T00:00:00Z, but this test fail due to the bug described in #160 hence it's ignored until we fix it

{
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(MAXIMUM_ALLOWED_FRACTIONAL_MILLIS, PST_OFFSET);
checkFields(9999, 12, 31, 15, 59, 59, new BigDecimal("0.999999999999"), PST_OFFSET, SECOND, ts);
assertEquals("9999-12-31T15:59:59.999999999999-08:00", ts.toString());
assertEquals("9999-12-31T23:59:59.999999999999Z", 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 + 1
Timestamp.forMillis(MAXIMUM_ALLOWED_FRACTIONAL_MILLIS.add(BigDecimal.ONE), PST_OFFSET);
}

@Test(expected = IllegalArgumentException.class)
public void testNewTimestampFromBigDecimalWithScaleTooBigPositive()
{
Timestamp.forMillis(new BigDecimal("1e100000"), PST_OFFSET);
}

@Test(expected = IllegalArgumentException.class)
public void testNewTimestampFromBigDecimalWithScaleTooBigNegative()
{
Timestamp.forMillis(new BigDecimal("1e-100000"), PST_OFFSET);
}

/**
Expand Down

0 comments on commit c3d4828

Please sign in to comment.