-
Notifications
You must be signed in to change notification settings - Fork 15
Handle large numbers in filter values #253
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
Open
Open
Changes from all commits
Commits
Show all changes
2 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -17,10 +17,12 @@ | |
| package ffapi | ||
|
|
||
| import ( | ||
| "bytes" | ||
| "context" | ||
| "database/sql/driver" | ||
| "encoding/json" | ||
| "fmt" | ||
| "math/big" | ||
| "strconv" | ||
| "strings" | ||
|
|
||
|
|
@@ -146,13 +148,24 @@ func SkipFieldValidation() *JSONBuildFilterOpt { | |
|
|
||
| func (js *SimpleFilterValue) UnmarshalJSON(b []byte) error { | ||
| var v interface{} | ||
| err := json.Unmarshal(b, &v) | ||
| d := json.NewDecoder(bytes.NewReader(b)) | ||
| d.UseNumber() | ||
| err := d.Decode(&v) | ||
| if err != nil { | ||
| return err | ||
| } | ||
| switch vi := v.(type) { | ||
| case float64: | ||
| *js = (SimpleFilterValue)(strconv.FormatFloat(vi, 'f', -1, 64)) | ||
| case json.Number: | ||
| // Parsed exactly (rather than as a float64) so large values such as a uint256 are not | ||
| // rounded, and bounded to 256 bits so an exponent cannot expand into an enormous string | ||
| if !boundedLiteral(vi.String()) { | ||
| return i18n.NewError(context.Background(), i18n.MsgJSONQueryValueUnsupported, string(b)) | ||
| } | ||
| r, ok := new(big.Rat).SetString(vi.String()) // #nosec G113 - length and exponent bounded by boundedLiteral above | ||
| if !ok || r.Num().BitLen() > maxFilterNumberBits || r.Denom().BitLen() > maxFilterNumberBits { | ||
| return i18n.NewError(context.Background(), i18n.MsgJSONQueryValueUnsupported, string(b)) | ||
| } | ||
| *js = (SimpleFilterValue)(formatDecimal(r)) | ||
| return nil | ||
| case string: | ||
| *js = (SimpleFilterValue)(vi) | ||
|
|
@@ -165,6 +178,40 @@ func (js *SimpleFilterValue) UnmarshalJSON(b []byte) error { | |
| } | ||
| } | ||
|
|
||
| const ( | ||
| maxFilterNumberBits = 256 | ||
| maxFilterNumberDigits = 78 // decimal digits in 2^256 | ||
| maxFilterNumberLength = maxFilterNumberDigits + maxFilterNumberBits + 8 // integer digits, decimal places (2^-255 needs 255), sign, point and exponent | ||
| ) | ||
|
|
||
| // boundedLiteral reports whether s, a valid JSON number, is short enough and has a small enough | ||
| // exponent that the value might fit in maxFilterNumberBits. | ||
| // Check before parsing to big.Rat (per gosec G113) | ||
| func boundedLiteral(s string) bool { | ||
| if len(s) > maxFilterNumberLength { | ||
| return false | ||
| } | ||
| i := strings.IndexAny(s, "eE") | ||
| if i < 0 { | ||
| return true | ||
| } | ||
| exp, err := strconv.Atoi(s[i+1:]) | ||
| limit := maxFilterNumberLength + maxFilterNumberDigits | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. you could move this limit to a const above |
||
| return err == nil && exp >= -limit && exp <= limit | ||
| } | ||
|
|
||
| // formatDecimal renders r, which was parsed from a decimal literal, as a plain decimal with | ||
| // no exponent and no trailing zeros (so "5.0" and "1e3" arrive as "5" and "1000") | ||
| func formatDecimal(r *big.Rat) string { | ||
| if r.IsInt() { | ||
| return r.Num().String() | ||
| } | ||
| // The denominator is 2^a*5^b, which terminates within max(a,b) places - and BitLen | ||
| // is always at least that | ||
| s := r.FloatString(r.Denom().BitLen()) | ||
| return strings.TrimRight(s, "0") | ||
| } | ||
|
|
||
| func (js SimpleFilterValue) String() string { | ||
| return (string)(js) | ||
| } | ||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -21,6 +21,7 @@ import ( | |
| "database/sql/driver" | ||
| "encoding/json" | ||
| "fmt" | ||
| "strings" | ||
| "testing" | ||
|
|
||
| "github.com/stretchr/testify/assert" | ||
|
|
@@ -777,3 +778,45 @@ func TestBuildQueryAndFail(t *testing.T) { | |
| _, err = qf.BuildFilter(context.Background(), TestQueryFactory) | ||
| assert.Regexp(t, "FF00142.*color", err) | ||
| } | ||
|
|
||
| func TestSimpleFilterValueNumbers(t *testing.T) { | ||
| for in, expected := range map[string]string{ | ||
| `9007199254740993`: "9007199254740993", // 2^53+1, not representable as a float64 | ||
| `115792089237316195423570985008687907853269984665640564039457584007913129639935`: "115792089237316195423570985008687907853269984665640564039457584007913129639935", // 2^256-1 | ||
| `-57896044618658097711785492504343953926634992332820282019728792003956564819968`: "-57896044618658097711785492504343953926634992332820282019728792003956564819968", // -2^255 | ||
| `-12345678901234567890123`: "-12345678901234567890123", | ||
| `42`: "42", | ||
| `5.0`: "5", | ||
| `1e3`: "1000", | ||
| `1.5`: "1.5", | ||
| `-0.000123`: "-0.000123", | ||
| `1.25e-2`: "0.0125", | ||
| `0.0009765625`: "0.0009765625", // 1/1024 - more places than denominator digits | ||
| `9007199254740993e0`: "9007199254740993", | ||
| `123456789012345678901.5`: "123456789012345678901.5", | ||
| `1e30`: "1000000000000000000000000000000", | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: worth adding an uppercase E test |
||
| // 2^-255, the most decimal places a value within 256 bits can need | ||
| `0.000000000000000000000000000000000000000000000000000000000000000000000000000017272337110188889250772703725600799142232000728872562770047406940337183606324854115943015006944576453121094587892299327193990197893663893387306007554116149549372494220733642578125`: "0.000000000000000000000000000000000000000000000000000000000000000000000000000017272337110188889250772703725600799142232000728872562770047406940337183606324854115943015006944576453121094587892299327193990197893663893387306007554116149549372494220733642578125", | ||
| `true`: "true", | ||
| `"str"`: "str", | ||
| } { | ||
| var js SimpleFilterValue | ||
| err := json.Unmarshal([]byte(in), &js) | ||
| assert.NoError(t, err, in) | ||
| assert.Equal(t, expected, js.String(), in) | ||
| } | ||
|
|
||
| var js SimpleFilterValue | ||
| err := js.UnmarshalJSON([]byte(`115792089237316195423570985008687907853269984665640564039457584007913129639936`)) // 2^256 | ||
| assert.Regexp(t, "FF00241", err) | ||
| err = js.UnmarshalJSON([]byte(`1e999999`)) | ||
| assert.Regexp(t, "FF00241", err) | ||
| err = js.UnmarshalJSON([]byte(`1e99999999999999999999`)) // exponent overflows an int | ||
| assert.Regexp(t, "FF00241", err) | ||
| err = js.UnmarshalJSON([]byte(`1e80`)) // within the exponent bound, but over 256 bits | ||
| assert.Regexp(t, "FF00241", err) | ||
| err = js.UnmarshalJSON([]byte(strings.Repeat("9", 1_000_000))) // too long to parse | ||
| assert.Regexp(t, "FF00241", err) | ||
| err = js.UnmarshalJSON([]byte(`1e-400`)) | ||
| assert.Regexp(t, "FF00241", err) | ||
| } | ||
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
note that is safe after json.Number conversion, I believe a hex number would still pass this such as
0x1p999999999and result in gosec 113. json.Number explicitly doesn't allow that hex format