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

Fix a panic in Postgres revision parsing for incompatible ZedTokens #1540

Merged
merged 1 commit into from
Sep 19, 2023
Merged
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
15 changes: 15 additions & 0 deletions internal/datastore/postgres/revisions.go
Original file line number Diff line number Diff line change
Expand Up @@ -169,6 +169,14 @@ func parseRevisionProto(revisionStr string) (datastore.Revision, error) {
}, nil
}

// MaxLegacyXIPDelta is the maximum allowed delta between the xmin and
// xmax revisions IDs on a *legacy* revision stored as a revision decimal.
// This is set to prevent a delta that is too large from blowing out the
// memory usage of the allocated slice, or even causing a panic in the case
// of a VERY large delta (which can be produced by, for example, a CRDB revision
// being given to a Postgres datastore accidentally).
const MaxLegacyXIPDelta = 1000

// parseRevisionDecimal parses a deprecated decimal.Decimal encoding of the revision
// with an optional xmin component, in the format of revision.xmin, e.g. 100.99.
// Because we're encoding to a snapshot, we want the revision to be considered visible,
Expand Down Expand Up @@ -203,6 +211,13 @@ func parseRevisionDecimal(revisionStr string) (datastore.Revision, error) {

var xipList []uint64
if xmax > xmin {
// Ensure that the delta is not too large to cause memory issues or a panic.
if xmax-xmin > MaxLegacyXIPDelta {
return nil, fmt.Errorf("received revision delta in excess of that expected; are you sure you're not passing a ZedToken from an incompatible datastore?")
}

// TODO(jschorr): Remove this deprecated code path at some point and maybe look into
// a more memory-efficient encoding of the XIP list if necessary.
xipList = make([]uint64, 0, xmax-xmin)
for i := xmin; i < xid; i++ {
xipList = append(xipList, i)
Expand Down
5 changes: 5 additions & 0 deletions internal/datastore/postgres/revisions_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -162,6 +162,11 @@ func TestCombinedRevisionParsing(t *testing.T) {
}
}

func TestBrokenInvalidRevision(t *testing.T) {
_, err := parseRevision("1693540940373045727.0000000001")
require.Error(t, err)
}

func FuzzRevision(f *testing.F) {
// Attempt to find a decimal revision that is a valid base64 encoded proto revision
f.Add(uint64(0), -1)
Expand Down
12 changes: 12 additions & 0 deletions pkg/zedtoken/zedtoken_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,18 @@ var decodeTests = []struct {
expectedRevision: decimal.New(12345, -2),
expectError: false,
},
{
format: "V1 ZedToken",
token: "GiAKHjE2OTM1NDA5NDAzNzMwNDU3MjcuMDAwMDAwMDAwMQ==",
expectedRevision: (func() decimal.Decimal {
v, err := decimal.NewFromString("1693540940373045727.0000000001")
if err != nil {
panic(err)
}
return v
})(),
expectError: false,
},
}

func TestDecode(t *testing.T) {
Expand Down