Skip to content

Commit

Permalink
Invalid UID checks (#5243) (#5252)
Browse files Browse the repository at this point in the history
Fixes #DGRAPH-1272
Fixes #5238

(cherry-picked from commit 09b2def
  • Loading branch information
parasssh committed Apr 20, 2020
1 parent ddad5fa commit 6b57f23
Show file tree
Hide file tree
Showing 9 changed files with 41 additions and 9 deletions.
6 changes: 6 additions & 0 deletions dgraph/cmd/bulk/mapper.go
Expand Up @@ -206,10 +206,16 @@ func (m *mapper) addMapEntry(key []byte, p *pb.Posting, shard int) {

func (m *mapper) processNQuad(nq gql.NQuad) {
sid := m.uid(nq.GetSubject())
if sid == 0 {
panic(fmt.Sprintf("invalid UID with value 0 for %v", nq.GetSubject()))
}
var oid uint64
var de *pb.DirectedEdge
if nq.GetObjectValue() == nil {
oid = m.uid(nq.GetObjectId())
if oid == 0 {
panic(fmt.Sprintf("invalid UID with value 0 for %v", nq.GetObjectId()))
}
de = nq.CreateUidEdge(sid, oid)
} else {
var err error
Expand Down
1 change: 1 addition & 0 deletions ee/backup/backup.go
Expand Up @@ -145,6 +145,7 @@ func (pr *Processor) WriteBackup(ctx context.Context) (*pb.Status, error) {
stream.ChooseKey = func(item *badger.Item) bool {
parsedKey, err := x.Parse(item.Key())
if err != nil {
glog.Errorf("error %v while parsing key %v during backup. Skip.", err, hex.EncodeToString(item.Key()))
return false
}

Expand Down
1 change: 0 additions & 1 deletion ee/backup/backup_oss.go
Expand Up @@ -10,7 +10,6 @@
* https://github.com/dgraph-io/dgraph/blob/master/licenses/DCL.txt
*/


package backup

// BadgerKeyFile - This is a copy of worker.Config.BadgerKeyFile. Need to copy because
Expand Down
2 changes: 1 addition & 1 deletion gql/mutation.go
Expand Up @@ -27,7 +27,7 @@ import (
)

var (
errInvalidUID = errors.New("UID has to be greater than one")
errInvalidUID = errors.New("UID must to be greater than 0")
)

// Mutation stores the strings corresponding to set and delete operations.
Expand Down
5 changes: 3 additions & 2 deletions posting/index.go
Expand Up @@ -91,9 +91,10 @@ func (txn *Txn) addIndexMutations(ctx context.Context, info *indexMutationInfo)

attr := info.edge.Attr
uid := info.edge.Entity
x.AssertTrue(uid != 0)
if uid == 0 {
return errors.New("invalid UID with value 0")
}
tokens, err := indexTokens(info)

if err != nil {
// This data is not indexable
return err
Expand Down
1 change: 1 addition & 0 deletions worker/draft.go
Expand Up @@ -132,6 +132,7 @@ func detectPendingTxns(attr string) error {
tctxs := posting.Oracle().IterateTxns(func(key []byte) bool {
pk, err := x.Parse(key)
if err != nil {
glog.Errorf("error %v while parsing key %v", err, hex.EncodeToString(key))
return false
}
return pk.Attr == attr
Expand Down
3 changes: 3 additions & 0 deletions worker/export.go
Expand Up @@ -20,6 +20,7 @@ import (
"bufio"
"bytes"
"compress/gzip"
"encoding/hex"
"encoding/json"
"fmt"
"os"
Expand Down Expand Up @@ -439,6 +440,7 @@ func export(ctx context.Context, in *pb.ExportRequest) error {
stream.ChooseKey = func(item *badger.Item) bool {
pk, err := x.Parse(item.Key())
if err != nil {
glog.Errorf("error %v while parsing key %v during export. Skip.", err, hex.EncodeToString(item.Key()))
return false
}

Expand All @@ -463,6 +465,7 @@ func export(ctx context.Context, in *pb.ExportRequest) error {
item := itr.Item()
pk, err := x.Parse(item.Key())
if err != nil {
glog.Errorf("error %v while parsing key %v during export. Skip.", err, hex.EncodeToString(item.Key()))
return nil, err
}
e := &exporter{
Expand Down
8 changes: 7 additions & 1 deletion x/keys.go
Expand Up @@ -471,6 +471,10 @@ func GetSplitKey(baseKey []byte, startUid uint64) ([]byte, error) {
func Parse(key []byte) (ParsedKey, error) {
var p ParsedKey

if len(key) == 0 {
return p, errors.New("0 length key")
}

p.bytePrefix = key[0]
if p.bytePrefix == ByteUnused {
return p, nil
Expand Down Expand Up @@ -500,7 +504,9 @@ func Parse(key []byte) (ParsedKey, error) {
return p, errors.Errorf("uid length < 8 for key: %q, parsed key: %+v", key, p)
}
p.Uid = binary.BigEndian.Uint64(k)

if p.Uid == 0 {
return p, errors.Errorf("Invalid UID with value 0 for key: %v", key)
}
if !p.HasStartUid {
break
}
Expand Down
23 changes: 19 additions & 4 deletions x/keys_test.go
Expand Up @@ -27,7 +27,14 @@ import (

func TestDataKey(t *testing.T) {
var uid uint64
for uid = 0; uid < 1001; uid++ {

// key with uid = 0 is invalid
uid = 0
key := DataKey("bad uid", uid)
_, err := Parse(key)
require.Error(t, err)

for uid = 1; uid < 1001; uid++ {
// Use the uid to derive the attribute so it has variable length and the test
// can verify that multiple sizes of attr work correctly.
sattr := fmt.Sprintf("attr:%d", uid)
Expand Down Expand Up @@ -58,7 +65,7 @@ func TestDataKey(t *testing.T) {
func TestParseDataKeysWithStartUid(t *testing.T) {
var uid uint64
startUid := uint64(math.MaxUint64)
for uid = 0; uid < 1001; uid++ {
for uid = 1; uid < 1001; uid++ {
sattr := fmt.Sprintf("attr:%d", uid)
key := DataKey(sattr, uid)
key, err := GetSplitKey(key, startUid)
Expand Down Expand Up @@ -113,7 +120,7 @@ func TestIndexKeyWithStartUid(t *testing.T) {

func TestReverseKey(t *testing.T) {
var uid uint64
for uid = 0; uid < 1001; uid++ {
for uid = 1; uid < 1001; uid++ {
sattr := fmt.Sprintf("attr:%d", uid)

key := ReverseKey(sattr, uid)
Expand All @@ -129,7 +136,7 @@ func TestReverseKey(t *testing.T) {
func TestReverseKeyWithStartUid(t *testing.T) {
var uid uint64
startUid := uint64(math.MaxUint64)
for uid = 0; uid < 1001; uid++ {
for uid = 1; uid < 1001; uid++ {
sattr := fmt.Sprintf("attr:%d", uid)

key := ReverseKey(sattr, uid)
Expand Down Expand Up @@ -208,3 +215,11 @@ func TestTypeKey(t *testing.T) {
require.Equal(t, sattr, pk.Attr)
}
}

func TestBadKeys(t *testing.T) {
// key with uid = 0 is invalid
uid := 0
key := DataKey("bad uid", uint64(uid))
_, err := Parse(key)
require.Error(t, err)
}

0 comments on commit 6b57f23

Please sign in to comment.