Add limits when deserializing BigDecimal and BigInteger (google#2510

) * Add limits when deserializing `BigDecimal` and `BigInteger` * Use assertThrows * Don't check number limits in JsonReader
getuniverse · Aug 17, 2024 · 74747ea · 74747ea
1 parent 34f9468
commit 74747ea
Show file tree

Hide file tree

Showing 7 changed files with 284 additions and 182 deletions.
diff --git a/gson/src/main/java/com/google/gson/JsonPrimitive.java b/gson/src/main/java/com/google/gson/JsonPrimitive.java
@@ -17,6 +17,7 @@
 package com.google.gson;
 
 import com.google.gson.internal.LazilyParsedNumber;
+import com.google.gson.internal.NumberLimits;
 import java.math.BigDecimal;
 import java.math.BigInteger;
 import java.util.Objects;
@@ -172,7 +173,7 @@ public double getAsDouble() {
    */
   @Override
   public BigDecimal getAsBigDecimal() {
-    return value instanceof BigDecimal ? (BigDecimal) value : new BigDecimal(getAsString());
+    return value instanceof BigDecimal ? (BigDecimal) value : NumberLimits.parseBigDecimal(getAsString());
   }
 
   /**
@@ -184,7 +185,7 @@ public BigInteger getAsBigInteger() {
         ? (BigInteger) value
         : isIntegral(this)
             ? BigInteger.valueOf(this.getAsNumber().longValue())
-            : new BigInteger(this.getAsString());
+            : NumberLimits.parseBigInteger(this.getAsString());
   }
 
   /**

diff --git a/gson/src/main/java/com/google/gson/ToNumberPolicy.java b/gson/src/main/java/com/google/gson/ToNumberPolicy.java
@@ -17,6 +17,7 @@
 package com.google.gson;
 
 import com.google.gson.internal.LazilyParsedNumber;
+import com.google.gson.internal.NumberLimits;
 import com.google.gson.stream.JsonReader;
 import com.google.gson.stream.MalformedJsonException;
 import java.io.IOException;
@@ -89,7 +90,7 @@ public enum ToNumberPolicy implements ToNumberStrategy {
     @Override public BigDecimal readNumber(JsonReader in) throws IOException {
       String value = in.nextString();
       try {
-        return new BigDecimal(value);
+        return NumberLimits.parseBigDecimal(value);
       } catch (NumberFormatException e) {
         throw new JsonParseException("Cannot parse " + value + "; at path " + in.getPreviousPath(), e);
       }

diff --git a/gson/src/main/java/com/google/gson/internal/LazilyParsedNumber.java b/gson/src/main/java/com/google/gson/internal/LazilyParsedNumber.java
@@ -35,6 +35,10 @@ public LazilyParsedNumber(String value) {
     this.value = value;
   }
 
+  private BigDecimal asBigDecimal() {
+    return NumberLimits.parseBigDecimal(value);
+  }
+
   @Override
   public int intValue() {
     try {
@@ -43,7 +47,7 @@ public int intValue() {
       try {
         return (int) Long.parseLong(value);
       } catch (NumberFormatException nfe) {
-        return new BigDecimal(value).intValue();
+        return asBigDecimal().intValue();
       }
     }
   }
@@ -53,7 +57,7 @@ public long longValue() {
     try {
       return Long.parseLong(value);
     } catch (NumberFormatException e) {
-      return new BigDecimal(value).longValue();
+      return asBigDecimal().longValue();
     }
   }
 
@@ -78,7 +82,7 @@ public String toString() {
    * deserialize it.
    */
   private Object writeReplace() throws ObjectStreamException {
-    return new BigDecimal(value);
+    return asBigDecimal();
   }
 
   private void readObject(ObjectInputStream in) throws IOException {

diff --git a/gson/src/main/java/com/google/gson/internal/NumberLimits.java b/gson/src/main/java/com/google/gson/internal/NumberLimits.java
@@ -0,0 +1,37 @@
+package com.google.gson.internal;
+
+import java.math.BigDecimal;
+import java.math.BigInteger;
+
+/**
+ * This class enforces limits on numbers parsed from JSON to avoid potential performance
+ * problems when extremely large numbers are used.
+ */
+public class NumberLimits {
+  private NumberLimits() {
+  }
+
+  private static final int MAX_NUMBER_STRING_LENGTH = 10_000;
+
+  private static void checkNumberStringLength(String s) {
+    if (s.length() > MAX_NUMBER_STRING_LENGTH) {
+      throw new NumberFormatException("Number string too large: " + s.substring(0, 30) + "...");
+    }
+  }
+
+  public static BigDecimal parseBigDecimal(String s) throws NumberFormatException {
+    checkNumberStringLength(s);
+    BigDecimal decimal = new BigDecimal(s);
+
+    // Cast to long to avoid issues with abs when value is Integer.MIN_VALUE
+    if (Math.abs((long) decimal.scale()) >= 10_000) {
+      throw new NumberFormatException("Number has unsupported scale: " + s);
+    }
+    return decimal;
+  }
+
+  public static BigInteger parseBigInteger(String s) throws NumberFormatException {
+    checkNumberStringLength(s);
+    return new BigInteger(s);
+  }
+}
diff --git a/gson/src/main/java/com/google/gson/internal/bind/TypeAdapters.java b/gson/src/main/java/com/google/gson/internal/bind/TypeAdapters.java
@@ -32,6 +32,7 @@
 import com.google.gson.annotations.SerializedName;
 import com.google.gson.internal.InvalidStateException;
 import com.google.gson.internal.LazilyParsedNumber;
+import com.google.gson.internal.NumberLimits;
 import com.google.gson.internal.TroubleshootingGuide;
 import com.google.gson.reflect.TypeToken;
 import com.google.gson.stream.JsonReader;
@@ -441,7 +442,7 @@ public void write(JsonWriter out, String value) throws IOException {
       }
       String s = in.nextString();
       try {
-        return new BigDecimal(s);
+        return NumberLimits.parseBigDecimal(s);
       } catch (NumberFormatException e) {
         throw new JsonSyntaxException("Failed parsing '" + s + "' as BigDecimal; at path " + in.getPreviousPath(), e);
       }
@@ -460,7 +461,7 @@ public void write(JsonWriter out, String value) throws IOException {
       }
       String s = in.nextString();
       try {
-        return new BigInteger(s);
+        return NumberLimits.parseBigInteger(s);
       } catch (NumberFormatException e) {
         throw new JsonSyntaxException("Failed parsing '" + s + "' as BigInteger; at path " + in.getPreviousPath(), e);
       }

diff --git a/gson/src/test/java/com/google/gson/functional/NumberLimitsTest.java b/gson/src/test/java/com/google/gson/functional/NumberLimitsTest.java
@@ -0,0 +1,185 @@
+package com.google.gson.functional;
+
+import static com.google.common.truth.Truth.assertThat;
+import static org.junit.Assert.assertThrows;
+
+import com.google.gson.Gson;
+import com.google.gson.JsonParseException;
+import com.google.gson.JsonPrimitive;
+import com.google.gson.JsonSyntaxException;
+import com.google.gson.ToNumberPolicy;
+import com.google.gson.ToNumberStrategy;
+import com.google.gson.TypeAdapter;
+import com.google.gson.internal.LazilyParsedNumber;
+import com.google.gson.stream.JsonReader;
+import com.google.gson.stream.JsonToken;
+import com.google.gson.stream.MalformedJsonException;
+import java.io.IOException;
+import java.io.ObjectOutputStream;
+import java.io.OutputStream;
+import java.io.StringReader;
+import java.math.BigDecimal;
+import java.math.BigInteger;
+import org.junit.Test;
+
+public class NumberLimitsTest {
+  private static final int MAX_LENGTH = 10_000;
+
+  private static JsonReader jsonReader(String json) {
+    return new JsonReader(new StringReader(json));
+  }
+
+  /**
+   * Tests how {@link JsonReader} behaves for large numbers.
+   *
+   * <p>Currently {@link JsonReader} itself does not enforce any limits.
+   * The reasons for this are:
+   * <ul>
+   *   <li>Methods such as {@link JsonReader#nextDouble()} seem to have no problem
+   *       parsing extremely large or small numbers (it rounds to 0 or Infinity)
+   *       (to be verified?; if it had performance problems with certain numbers, then
+   *        it would affect other parts of Gson which parse as float or double as well)
+   *   <li>Enforcing limits only when a JSON number is encountered would be ineffective
+   *       unless users explicitly call {@link JsonReader#peek()} and check if the value
+   *       is a JSON number. Otherwise the limits could be circumvented because
+   *       {@link JsonReader#nextString()} reads both strings and numbers, and for
+   *       JSON strings no restrictions are enforced.
+   * </ul>
+   */
+  @Test
+  public void testJsonReader() throws IOException {
+    JsonReader reader = jsonReader("1".repeat(1000));
+    assertThat(reader.peek()).isEqualTo(JsonToken.NUMBER);
+    assertThat(reader.nextString()).isEqualTo("1".repeat(1000));
+
+    JsonReader reader2 = jsonReader("1".repeat(MAX_LENGTH + 1));
+    // Currently JsonReader does not recognize large JSON numbers as numbers but treats them
+    // as unquoted string
+    MalformedJsonException e = assertThrows(MalformedJsonException.class, () -> reader2.peek());
+    assertThat(e).hasMessageThat().startsWith("Use JsonReader.setStrictness(Strictness.LENIENT) to accept malformed JSON");
+
+    reader = jsonReader("1e9999");
+    assertThat(reader.peek()).isEqualTo(JsonToken.NUMBER);
+    assertThat(reader.nextString()).isEqualTo("1e9999");
+
+    reader = jsonReader("1e+9999");
+    assertThat(reader.peek()).isEqualTo(JsonToken.NUMBER);
+    assertThat(reader.nextString()).isEqualTo("1e+9999");
+
+    reader = jsonReader("1e10000");
+    assertThat(reader.peek()).isEqualTo(JsonToken.NUMBER);
+    assertThat(reader.nextString()).isEqualTo("1e10000");
+
+    reader = jsonReader("1e00001");
+    assertThat(reader.peek()).isEqualTo(JsonToken.NUMBER);
+    assertThat(reader.nextString()).isEqualTo("1e00001");
+  }
+
+  @Test
+  public void testJsonPrimitive() {
+    assertThat(new JsonPrimitive("1".repeat(MAX_LENGTH)).getAsBigDecimal())
+        .isEqualTo(new BigDecimal("1".repeat(MAX_LENGTH)));
+    assertThat(new JsonPrimitive("1e9999").getAsBigDecimal())
+        .isEqualTo(new BigDecimal("1e9999"));
+    assertThat(new JsonPrimitive("1e-9999").getAsBigDecimal())
+        .isEqualTo(new BigDecimal("1e-9999"));
+
+    NumberFormatException e = assertThrows(NumberFormatException.class,
+        () -> new JsonPrimitive("1".repeat(MAX_LENGTH + 1)).getAsBigDecimal());
+    assertThat(e).hasMessageThat().isEqualTo("Number string too large: 111111111111111111111111111111...");
+
+    e = assertThrows(NumberFormatException.class,
+        () -> new JsonPrimitive("1e10000").getAsBigDecimal());
+    assertThat(e).hasMessageThat().isEqualTo("Number has unsupported scale: 1e10000");
+
+    e = assertThrows(NumberFormatException.class,
+        () -> new JsonPrimitive("1e-10000").getAsBigDecimal());
+    assertThat(e).hasMessageThat().isEqualTo("Number has unsupported scale: 1e-10000");
+
+
+    assertThat(new JsonPrimitive("1".repeat(MAX_LENGTH)).getAsBigInteger())
+        .isEqualTo(new BigInteger("1".repeat(MAX_LENGTH)));
+
+    e = assertThrows(NumberFormatException.class,
+        () -> new JsonPrimitive("1".repeat(MAX_LENGTH + 1)).getAsBigInteger());
+    assertThat(e).hasMessageThat().isEqualTo("Number string too large: 111111111111111111111111111111...");
+  }
+
+  @Test
+  public void testToNumberPolicy() throws IOException {
+    ToNumberStrategy strategy = ToNumberPolicy.BIG_DECIMAL;
+
+    assertThat(strategy.readNumber(jsonReader("\"" + "1".repeat(MAX_LENGTH) + "\"")))
+        .isEqualTo(new BigDecimal("1".repeat(MAX_LENGTH)));
+    assertThat(strategy.readNumber(jsonReader("1e9999")))
+        .isEqualTo(new BigDecimal("1e9999"));
+
+
+    JsonParseException e = assertThrows(JsonParseException.class,
+        () -> strategy.readNumber(jsonReader("\"" + "1".repeat(MAX_LENGTH + 1) + "\"")));
+    assertThat(e).hasMessageThat().isEqualTo("Cannot parse " + "1".repeat(MAX_LENGTH + 1) + "; at path $");
+    assertThat(e).hasCauseThat().hasMessageThat().isEqualTo("Number string too large: 111111111111111111111111111111...");
+
+    e = assertThrows(JsonParseException.class, () -> strategy.readNumber(jsonReader("\"1e10000\"")));
+    assertThat(e).hasMessageThat().isEqualTo("Cannot parse 1e10000; at path $");
+    assertThat(e).hasCauseThat().hasMessageThat().isEqualTo("Number has unsupported scale: 1e10000");
+  }
+
+  @Test
+  public void testLazilyParsedNumber() throws IOException {
+    assertThat(new LazilyParsedNumber("1".repeat(MAX_LENGTH)).intValue())
+        .isEqualTo(new BigDecimal("1".repeat(MAX_LENGTH)).intValue());
+    assertThat(new LazilyParsedNumber("1e9999").intValue())
+        .isEqualTo(new BigDecimal("1e9999").intValue());
+
+    NumberFormatException e = assertThrows(NumberFormatException.class,
+        () -> new LazilyParsedNumber("1".repeat(MAX_LENGTH + 1)).intValue());
+    assertThat(e).hasMessageThat().isEqualTo("Number string too large: 111111111111111111111111111111...");
+
+    e = assertThrows(NumberFormatException.class,
+        () -> new LazilyParsedNumber("1e10000").intValue());
+    assertThat(e).hasMessageThat().isEqualTo("Number has unsupported scale: 1e10000");
+
+    e = assertThrows(NumberFormatException.class,
+        () -> new LazilyParsedNumber("1e10000").longValue());
+    assertThat(e).hasMessageThat().isEqualTo("Number has unsupported scale: 1e10000");
+
+    ObjectOutputStream objOut = new ObjectOutputStream(OutputStream.nullOutputStream());
+    // Number is serialized as BigDecimal; should also enforce limits during this conversion
+    e = assertThrows(NumberFormatException.class, () -> objOut.writeObject(new LazilyParsedNumber("1e10000")));
+    assertThat(e).hasMessageThat().isEqualTo("Number has unsupported scale: 1e10000");
+  }
+
+  @Test
+  public void testBigDecimalAdapter() throws IOException {
+    TypeAdapter<BigDecimal> adapter = new Gson().getAdapter(BigDecimal.class);
+
+    assertThat(adapter.fromJson("\"" + "1".repeat(MAX_LENGTH) + "\""))
+        .isEqualTo(new BigDecimal("1".repeat(MAX_LENGTH)));
+    assertThat(adapter.fromJson("\"1e9999\""))
+        .isEqualTo(new BigDecimal("1e9999"));
+
+    JsonSyntaxException e = assertThrows(JsonSyntaxException.class,
+        () -> adapter.fromJson("\"" + "1".repeat(MAX_LENGTH + 1) + "\""));
+    assertThat(e).hasMessageThat().isEqualTo("Failed parsing '" + "1".repeat(MAX_LENGTH + 1) + "' as BigDecimal; at path $");
+    assertThat(e).hasCauseThat().hasMessageThat().isEqualTo("Number string too large: 111111111111111111111111111111...");
+
+    e = assertThrows(JsonSyntaxException.class,
+        () -> adapter.fromJson("\"1e10000\""));
+    assertThat(e).hasMessageThat().isEqualTo("Failed parsing '1e10000' as BigDecimal; at path $");
+    assertThat(e).hasCauseThat().hasMessageThat().isEqualTo("Number has unsupported scale: 1e10000");
+  }
+
+  @Test
+  public void testBigIntegerAdapter() throws IOException {
+    TypeAdapter<BigInteger> adapter = new Gson().getAdapter(BigInteger.class);
+
+    assertThat(adapter.fromJson("\"" + "1".repeat(MAX_LENGTH) + "\""))
+        .isEqualTo(new BigInteger("1".repeat(MAX_LENGTH)));
+
+    JsonSyntaxException e = assertThrows(JsonSyntaxException.class,
+        () -> adapter.fromJson("\"" + "1".repeat(MAX_LENGTH + 1) + "\""));
+    assertThat(e).hasMessageThat().isEqualTo("Failed parsing '" + "1".repeat(MAX_LENGTH + 1) + "' as BigInteger; at path $");
+    assertThat(e).hasCauseThat().hasMessageThat().isEqualTo("Number string too large: 111111111111111111111111111111...");
+  }
+}