search

FasterXML / jackson-modules-java8

Set of support modules for Java 8 datatypes (Optionals, date/time) and features (parameter names)
Apache License 2.0
401 stars 117 forks source link

Performance issue with malicious `BigDecimal` input, `InstantDeserializer`, `DurationDeserializer` (CVE-2018-1000873) #90

Closed cowtowncoder closed 6 years ago

cowtowncoder commented 6 years ago

(note: moved from https://github.com/FasterXML/jackson-databind/issues/2141 reported by @plokhotnyuk)

It looks the same as: https://github.com/playframework/play-json/issues/180

Reproduced by the following commit: https://github.com/plokhotnyuk/jsoniter-scala/pull/153/commits/0d53faf5093b492867b550f2cec55ff0b5cc62de

The security bug is in InstantDeserializer and DurationDeserializer of the jackson-datatype-jsr310 artifact:

    protected T _fromDecimal(DeserializationContext context, BigDecimal value)
    {
        long seconds = value.longValue();   // <- hangs in case of 10000000e100000000 
        int nanoseconds = DecimalUtils.extractNanosecondDecimal(value, seconds);
        return fromNanoseconds.apply(new FromDecimalArguments(
                seconds, nanoseconds, getZone(context)));
    }

W/A is to use custom serializers for all types that are parsed with InstantDeserializer and DurationDeserializer by registering them after (or instead of) registration of the JavaTimeModule module.

cowtowncoder commented 6 years ago

(copied from above-mentioned original issue; comment by @plokhotnyuk)

Current implementations of java.math.BigInteger and java.math.BigDecimal in Java 8+ use binary (base-2) representation and during parsing translate provided decimal (base-10) representation to binary one:

https://github.com/AdoptOpenJDK/openjdk-jdk8u/blob/52fdb973403401f4073df3849793e1415ca2bc93/jdk/src/share/classes/java/math/BigInteger.java

https://github.com/AdoptOpenJDK/openjdk-jdk8u/blob/52fdb973403401f4073df3849793e1415ca2bc93/jdk/src/share/classes/java/math/BigDecimal.java

Below are results of benchmarks for different JSON parsers for Scala (including Jackson-module-scala) which are parametrized by the size parameter that specifies number of significant digits in scala.math.BigInt (which is just a wrapper over java.math.BigInteger):

[info] REMEMBER: The numbers below are just data. To gain reusable insights, you need to follow up on
[info] why the numbers are the way they are. Use profilers (see -prof, -lprof), design factorial
[info] experiments, perform baseline and negative tests that provide experimental control, make sure
[info] the benchmarking environment is safe on JVM/OS/HW level, ask for reviews from the domain experts.
[info] Do not assume the numbers tell you what you want them to tell.
[info] Benchmark                              (size)   Mode  Cnt         Score         Error  Units
[info] BigIntBenchmark.readAVSystemGenCodec        1  thrpt    5  14890392.293 ±  590175.129  ops/s
[info] BigIntBenchmark.readAVSystemGenCodec       10  thrpt    5   6771799.312 ±  258856.674  ops/s
[info] BigIntBenchmark.readAVSystemGenCodec      100  thrpt    5   1507693.582 ±   35178.285  ops/s
[info] BigIntBenchmark.readAVSystemGenCodec     1000  thrpt    5     56379.543 ±    1754.682  ops/s
[info] BigIntBenchmark.readAVSystemGenCodec    10000  thrpt    5       721.463 ±      22.906  ops/s
[info] BigIntBenchmark.readAVSystemGenCodec   100000  thrpt    5         7.452 ±       0.043  ops/s
[info] BigIntBenchmark.readAVSystemGenCodec  1000000  thrpt    5         0.078 ±       0.001  ops/s
[info] BigIntBenchmark.readCirce                   1  thrpt    5   9128874.145 ±  206652.546  ops/s
[info] BigIntBenchmark.readCirce                  10  thrpt    5   7261855.082 ±  357869.830  ops/s
[info] BigIntBenchmark.readCirce                 100  thrpt    5    446646.616 ±   14351.238  ops/s
[info] BigIntBenchmark.readCirce                1000  thrpt    5     20715.338 ±     303.567  ops/s
[info] BigIntBenchmark.readCirce               10000  thrpt    5       488.270 ±      32.506  ops/s
[info] BigIntBenchmark.readCirce              100000  thrpt    5         6.606 ±       0.487  ops/s
[info] BigIntBenchmark.readJacksonScala            1  thrpt    5   5203959.437 ±  288410.893  ops/s
[info] BigIntBenchmark.readJacksonScala           10  thrpt    5   2754737.373 ±  137166.533  ops/s
[info] BigIntBenchmark.readJacksonScala          100  thrpt    5    923723.753 ±   32844.855  ops/s
[info] BigIntBenchmark.readJacksonScala         1000  thrpt    5     47777.094 ±    2031.990  ops/s
[info] BigIntBenchmark.readJacksonScala        10000  thrpt    5       670.702 ±      27.796  ops/s
[info] BigIntBenchmark.readJacksonScala       100000  thrpt    5         7.170 ±       0.159  ops/s
[info] BigIntBenchmark.readJacksonScala      1000000  thrpt    5         0.075 ±       0.001  ops/s
[info] BigIntBenchmark.readJsoniterScala           1  thrpt    5  60184573.095 ± 1329210.574  ops/s
[info] BigIntBenchmark.readJsoniterScala          10  thrpt    5  30459292.738 ±  751977.834  ops/s
[info] BigIntBenchmark.readJsoniterScala         100  thrpt    5   1384704.308 ±   11719.557  ops/s
[info] BigIntBenchmark.readNaiveScala              1  thrpt    5  29224139.625 ± 2260565.546  ops/s
[info] BigIntBenchmark.readNaiveScala             10  thrpt    5  11295523.994 ±  204324.161  ops/s
[info] BigIntBenchmark.readNaiveScala            100  thrpt    5   2024505.047 ±   17078.738  ops/s
[info] BigIntBenchmark.readNaiveScala           1000  thrpt    5     62508.755 ±    1824.931  ops/s
[info] BigIntBenchmark.readNaiveScala          10000  thrpt    5       765.462 ±       5.213  ops/s
[info] BigIntBenchmark.readNaiveScala         100000  thrpt    5         7.770 ±       0.191  ops/s
[info] BigIntBenchmark.readNaiveScala        1000000  thrpt    5         0.077 ±       0.001  ops/s
[info] BigIntBenchmark.readPlayJson                1  thrpt    5   9070015.382 ±  671907.068  ops/s
[info] BigIntBenchmark.readPlayJson               10  thrpt    5   7680490.729 ±  193593.896  ops/s
[info] BigIntBenchmark.readPlayJson              100  thrpt    5   1177518.121 ±   96448.882  ops/s
[info] BigIntBenchmark.readPlayJson             1000  thrpt    5     56525.825 ±    1110.685  ops/s
[info] BigIntBenchmark.readPlayJson            10000  thrpt    5       746.515 ±       9.304  ops/s
[info] BigIntBenchmark.readPlayJson           100000  thrpt    5         7.724 ±       0.077  ops/s
[info] BigIntBenchmark.readPlayJson          1000000  thrpt    5         0.072 ±       0.012  ops/s

To run them on your JDK:

  1. Install latest version of sbt and/or ensure that it already installed properly: 

    sbt about

  2. Clone jsoniter-scala repo: 

    git clone https://github.com/plokhotnyuk/jsoniter-scala.git

  3. Enter to the cloned directory:

    cd jsoniter-scala

  4. Run benchmarks using a path parameter to your JDK:

    sbt -no-colors 'jsoniter-scala-benchmark/jmh:run -jvm /usr/lib/jvm/jdk-11/bin/java -wi 5 -i 5 .*BigIntBench.*read.*'
cowtowncoder commented 6 years ago

Fixed via #87, to be included in 2.9.8 and later.

ddillard commented 5 years ago

Any plans to get a CVE for this issue? If not, I can request one.

cowtowncoder commented 5 years ago

@ddillard Not aware of one. If you can do that, that'd be helpful!

ddillard commented 5 years ago

@cowtowncoder Ok, I'll submit a request and post here when it's approved. Usually takes a few weeks to get one and given the holidays I wouldn't be surprised if takes a bit longer.

ddillard commented 5 years ago

Good news! Kurt got to this really quickly and this issue has been assigned CVE-2018-1000873. Should get published in the NVD in the next few days.

datagitlies commented 5 years ago

@cowtowncoder are there any plans to fix this CVE in the 2.8 branch and cut a new release there? I'd volunteer to submit a pull request as #87 didn't look too complex. However, I can't help with the release part. Thoughts?

cowtowncoder commented 5 years ago

@datagitlies No plans to backport as I really want to close 2.8 branch and any time spent on releasing from pre-2.9 is away from limited amount of time I have for my OSS hobby. In this case patch would need to involve multiple components, for general DoS protection (jackson-core and jackson-databind have some, as well as this module). So, I appreciate your offer to help, but I don't think I want to work on backport here.