price: Allow more decimal numbers in price.Parse

[bartekn]

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:

go/price/main.go

Lines 20 to 26 in 04ac85c

	// validAmountSimple is a simple regular expression checking if a string looks like
	// a number, more or less. The details will be checked in `math/big` internally.
	// What we want to prevent is passing very big numbers like `1e9223372036854775807`
	// to `big.Rat.SetString` triggering long calculations.
	// Note: {1,20} because the biggest amount you can use in Stellar is:
	// len("922337203685.4775807") = 20.
	validAmountSimple = regexp.MustCompile("^-?[.0-9]{1,20}$")

go/price/main.go

Lines 41 to 47 in 04ac85c

	// continuedFraction calculates and returns the best rational approximation of
	// the given real number.
	func continuedFraction(price string) (xdrPrice xdr.Price, err error) {
	if !validAmountSimple.MatchString(price) {
	return xdrPrice, fmt.Errorf("invalid price format: %s", price)
	}

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]

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.