New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CVE-2018-18853 Denial of service when parsing a JSON object with an unexpected field that has a big number #278

Closed
plokhotnyuk opened this Issue Oct 15, 2018 · 0 comments

Comments

Projects
None yet
2 participants
@plokhotnyuk

plokhotnyuk commented Oct 15, 2018

Sub-quadratic decreasing of throughput when length of the JSON object is increasing

On contemporary CPUs parsing of such JSON object with an additional field that has of 1000000 decimal digits (~1Mb) can took more than 14 seconds:

[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] ExtractFieldsBenchmark.readSpray        1  thrpt    5  2295616.947 ± 156189.629  ops/s
[info] ExtractFieldsBenchmark.readSpray       10  thrpt    5  2259250.907 ±  73227.696  ops/s
[info] ExtractFieldsBenchmark.readSpray      100  thrpt    5   604608.260 ±   2896.206  ops/s
[info] ExtractFieldsBenchmark.readSpray     1000  thrpt    5    36803.177 ±   2190.794  ops/s
[info] ExtractFieldsBenchmark.readSpray    10000  thrpt    5      558.418 ±     32.686  ops/s
[info] ExtractFieldsBenchmark.readSpray   100000  thrpt    5        6.466 ±      0.199  ops/s
[info] ExtractFieldsBenchmark.readSpray  1000000  thrpt    5        0.071 ±      0.004  ops/s

Reproducible Test Case

To run that benchmarks on your JDK:

  1. Install latest version of sbt and/or ensure that it already installed properly:
sbt about
  1. Clone jsoniter-scala repo:
git clone https://github.com/plokhotnyuk/jsoniter-scala.git
  1. Enter to the cloned directory and checkout for the specific branch:
cd jsoniter-scala
git checkout spray-json-DoS-by-a-big-number
  1. 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 2 -i 5 .*ExtractFieldsBench.*Spray.*'

@jrudolph jrudolph added the Bug label Oct 30, 2018

jrudolph added a commit to jrudolph/spray-json that referenced this issue Oct 30, 2018

Limit the number of characters for numbers in the parser, fixes spray…
…#278

BigInteger/BigDecimal seems to have approx. quadratic runtime for instantiating big numbers
from Strings. Lacking a better solution we introduce a character limit for
numbers. According to the benchmarks from spray#278, at 100 digits the constant/linear
parts still predominate over the quadratic slowdowns seen with 10000+ digits.

@jrudolph jrudolph added the Security label Nov 7, 2018

@jrudolph jrudolph changed the title from Denial of service when parsing a JSON object with an unexpected field that has a big number to CVE-2018-18853 Denial of service when parsing a JSON object with an unexpected field that has a big number Nov 7, 2018

jrudolph added a commit to jrudolph/spray-json that referenced this issue Nov 7, 2018

Limit the number of characters for numbers in the parser, fixes spray…
…#278

BigInteger/BigDecimal seems to have approx. quadratic runtime for instantiating big numbers
from Strings. Lacking a better solution we introduce a character limit for
numbers. According to the benchmarks from spray#278, at 100 digits the constant/linear
parts still predominate over the quadratic slowdowns seen with 10000+ digits.

@jrudolph jrudolph closed this in a8c45e7 Nov 8, 2018

jrudolph added a commit that referenced this issue Nov 8, 2018

Merge pull request #283 from jrudolph/limit-size-of-numbers
CVE-2018-18853 Limit the number of characters for numbers in the parser, fixes #278

@jrudolph jrudolph added this to the 1.3.5 milestone Nov 8, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment