Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
53 changes: 50 additions & 3 deletions pkg/ffapi/restfilter_json.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,10 +17,12 @@
package ffapi

import (
"bytes"
"context"
"database/sql/driver"
"encoding/json"
"fmt"
"math/big"
"strconv"
"strings"

Expand Down Expand Up @@ -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)
Expand All @@ -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 {

Copy link
Copy Markdown
Contributor

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 0x1p999999999 and result in gosec 113. json.Number explicitly doesn't allow that hex format

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

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The 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)
}
Expand Down
43 changes: 43 additions & 0 deletions pkg/ffapi/restfilter_json_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import (
"database/sql/driver"
"encoding/json"
"fmt"
"strings"
"testing"

"github.com/stretchr/testify/assert"
Expand Down Expand Up @@ -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",

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The 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)
}
Loading