Skip to content
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

Denial of service parsing JsNumber as Instant/ZonedDateTime/LocalDateTime #180

Closed
steinybot opened this issue Sep 18, 2018 · 7 comments
Closed

Comments

@steinybot
Copy link

Play JSON Version (2.5.x / etc)

2.6.9

API (Scala / Java / Neither / Both)

Scala 2.12.6

Operating System (Ubuntu 15.10 / MacOS 10.10 / Windows 10)

MacOS 10.13.6

JDK (Oracle 1.8.0_72, OpenJDK 1.8.x, Azul Zing)

Paste the output from java -version at the command line.

Library Dependencies

N/A

Expected Behavior

Run the following code:

  val number = BigDecimal(BigInt(Long.MaxValue), Int.MaxValue, MathContext.DECIMAL128)
  val jsNumber = JsNumber(number)
  val result = Reads.of[ZonedDateTime].reads(jsNumber)

It should return a result quickly.

Actual Behavior

It is CPU bound for an unknown period of time (I have never waited long enough for it to complete).

plokhotnyuk added a commit to plokhotnyuk/jsoniter-scala that referenced this issue Sep 18, 2018
plokhotnyuk added a commit to plokhotnyuk/jsoniter-scala that referenced this issue Sep 18, 2018
@OlegYch
Copy link
Contributor

OlegYch commented Sep 18, 2018

this hangs play.api.libs.json.Json.parse("1000000000e1000000000").as[BigDecimal].toLong

plokhotnyuk added a commit to plokhotnyuk/jsoniter-scala that referenced this issue Sep 18, 2018
@alexdupre
Copy link

Converting a huge BigDecimal to a long takes a lot of time, it's probably better to use toLongExact instead of toLong that immediately throws an error if it's not valid.

@plokhotnyuk
Copy link

It will be not fully compatible with the current implementation too

scala> BigDecimal("1.1").toLongExact
java.lang.ArithmeticException: Rounding necessary
  at java.math.BigDecimal.commonNeedIncrement(BigDecimal.java:4148)
  at java.math.BigDecimal.needIncrement(BigDecimal.java:4204)
  at java.math.BigDecimal.divideAndRound(BigDecimal.java:4112)
  at java.math.BigDecimal.setScale(BigDecimal.java:2452)
  at java.math.BigDecimal.longValueExact(BigDecimal.java:3090)
  at scala.math.BigDecimal.toLongExact(BigDecimal.scala:728)
  ... 28 elided

@alexdupre
Copy link

If we want to accept decimals to be backward compatible we can use setScale(0, RoundingMode.FLOOR).toLongExact

plokhotnyuk added a commit to plokhotnyuk/jsoniter-scala that referenced this issue Sep 18, 2018
@OlegYch
Copy link
Contributor

OlegYch commented Sep 18, 2018

@alexdupre that still hangs
i'm going to submit a pr which rejects non-long numbers

@steinybot
Copy link
Author

@OlegYch The fix for this isn't correct for Instant's as they are not just Long's.

Quoting from the JavaDoc:

The range of an instant requires the storage of a number larger than a long. To achieve this, the class stores a long representing epoch-seconds and an int representing nanosecond-of-second, which will always be between 0 and 999,999,999.

@steinybot
Copy link
Author

steinybot commented Sep 18, 2018

Hmm that is probably a separate issue. I don't think it ever handled Instant's properly (or at least not in the way I expected it to).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants