stellar / go

Stellar's public monorepo of go code
https://stellar.org/developers
Apache License 2.0
1.29k stars 503 forks source link

price: Allow more decimal numbers in `price.Parse` #2515

Open bartekn opened 4 years ago

bartekn commented 4 years ago

What version are you using?

master

What did you do?

price.Parse is using price.continuedFraction internally. It checks if the given price is a valid Stellar amount using regexp: https://github.com/stellar/go/blob/04ac85c94afe1fbb24e850601812f4c791926b24/price/main.go#L20-L26 https://github.com/stellar/go/blob/04ac85c94afe1fbb24e850601812f4c791926b24/price/main.go#L41-L47

This behaviour is incorrect. Requirement for amounts to have at max 7 decimal places comes from the way stellar-core keeps amount values internally (int64 in stroops which is 10-7 XLM). There is no such limitation on string representation of the price.

The regexp check appeared in the code around the time when we received security report about possible DoS vector when parsing amounts using amount.Parse function. The argument value was passed directly to big.Rat.SetString which can start extensive calculations on values like 1E9223372036854775807 (note: E). This behaviour was incorrectly incorporated into price.Parse because it's using big.Rat.SetString internally too.

When fixing this issue we should remember about this DoS vector. Specifically, we should check what's the maximum number of decimal places we can allow to ensure low CPU usage.

arkantos1482 commented 3 years ago

I encounter to different problem related to "price.parse".

I cannot enter more than MAX_INT (2,147,483,647) like 3 trillion as price value in buy/sell offer (it is common on offers between BTC and weak currencies). i believe it may be possible to have the (922337203685.4775807) as the top limit price value.