Reject requests with predicates larger than the max size allowed. (#3052)

* Reject requests with predicates larger than the max size allowed.

This change introduces checks in server.go to verify that Alter,
Mutation, and Query requests that includes predicates larger than 2^16
characters are rejected before being processed.

* Address review comments.

* run go fmt

Author: martinmr

edgraph/server.go

@@ -348,13 +348,17 @@ func (s *Server) Alter(ctx context.Context, op *api.Operation) (*api.Payload, er
 		return empty, err
 	}
 
-	// Reserved predicates cannot be altered.
 	for _, update := range result.Schemas {
+		// Reserved predicates cannot be altered.
 		if x.IsReservedPredicate(update.Predicate) {
 			err := fmt.Errorf("predicate %s is reserved and is not allowed to be modified",
 				update.Predicate)
 			return nil, err
 		}
+
+		if err := validatePredName(update.Predicate); err != nil {
+			return nil, err
+		}
 	}
 
 	glog.Infof("Got schema: %+v\n", result.Schemas)
@@ -560,6 +564,11 @@ func (s *Server) doQuery(ctx context.Context, req *api.Request) (resp *api.Respo
 	if err != nil {
 		return resp, err
 	}
+
+	if err = validateQuery(parsedReq.Query); err != nil {
+		return resp, err
+	}
+
 	if req.StartTs == 0 {
 		req.StartTs = State.getTimestamp(req.ReadOnly)
 	}
@@ -769,6 +778,9 @@ func validateAndConvertFacets(nquads []*api.NQuad) error {
 
 func validateNQuads(set, del []*api.NQuad) error {
 	for _, nq := range set {
+		if err := validatePredName(nq.Predicate); err != nil {
+			return err
+		}
 		var ostar bool
 		if o, ok := nq.ObjectValue.GetVal().(*api.Value_DefaultVal); ok {
 			ostar = o.DefaultVal == x.Star
@@ -781,6 +793,9 @@ func validateNQuads(set, del []*api.NQuad) error {
 		}
 	}
 	for _, nq := range del {
+		if err := validatePredName(nq.Predicate); err != nil {
+			return err
+		}
 		var ostar bool
 		if o, ok := nq.ObjectValue.GetVal().(*api.Value_DefaultVal); ok {
 			ostar = o.DefaultVal == x.Star
@@ -821,3 +836,27 @@ func validateKeys(nq *api.NQuad) error {
 	}
 	return nil
 }
+
+// validateQuery verifies that the query does not contain any preds that
+// are longer than the limit (2^16).
+func validateQuery(queries []*gql.GraphQuery) error {
+	for _, q := range queries {
+		if err := validatePredName(q.Attr); err != nil {
+			return err
+		}
+
+		if err := validateQuery(q.Children); err != nil {
+			return err
+		}
+	}
+
+	return nil
+}
+
+func validatePredName(name string) error {
+	if len(name) > math.MaxUint16 {
+		return fmt.Errorf("Predicate name length cannot be bigger than 2^16. Predicate: %v",
+			name[:80])
+	}
+	return nil
+}

query/query3_test.go

@@ -19,6 +19,8 @@ package query
 import (
 	"context"
 	"encoding/json"
+	"fmt"
+	"strings"
 	"testing"
 
 	"github.com/stretchr/testify/require"
@@ -1949,3 +1951,26 @@ func TestMultipleTypeDirectivesInPredicate(t *testing.T) {
 	js := processQueryNoErr(t, query)
 	require.JSONEq(t, `{"data": {"me":[{"enemy":[{"name":"Margaret", "pet":[{"name":"Bear"}]}, {"name":"Leonard"}]}]}}`, js)
 }
+
+func TestMaxPredicateSize(t *testing.T) {
+	// Create a string that has more than than 2^16 chars.
+	var b strings.Builder
+	for i := 0; i < 10000; i++ {
+		b.WriteString("abcdefg")
+	}
+	largePred := b.String()
+
+	query := fmt.Sprintf(`
+		{
+			me(func: uid(0x2)) {
+				%s {
+					name
+				}
+			}
+		}
+	`, largePred)
+
+	_, err := processQuery(t, context.Background(), query)
+	require.Error(t, err)
+	require.Contains(t, err.Error(), "Predicate name length cannot be bigger than 2^16")
+}

systest/mutations_test.go

@@ -79,6 +79,7 @@ func TestSystem(t *testing.T) {
 	t.Run("has should not have deleted edge", wrap(HasDeletedEdge))
 	t.Run("has should have reverse edges", wrap(HasReverseEdge))
 	t.Run("facet json input supports anyofterms query", wrap(FacetJsonInputSupportsAnyOfTerms))
+	t.Run("max predicate size", wrap(MaxPredicateSize))
 }
 
 func FacetJsonInputSupportsAnyOfTerms(t *testing.T, c *dgo.Dgraph) {
@@ -1678,3 +1679,40 @@ func HasReverseEdge(t *testing.T, c *dgo.Dgraph) {
 	require.Equal(t, len(revs), 1)
 	require.Equal(t, revs[0].Name, "carol")
 }
+
+func MaxPredicateSize(t *testing.T, c *dgo.Dgraph) {
+	// Create a string that has more than than 2^16 chars.
+	var b strings.Builder
+	for i := 0; i < 10000; i++ {
+		b.WriteString("abcdefg")
+	}
+	largePred := b.String()
+
+	// Verify that Alter requests with predicates that are too large are rejected.
+	ctx := context.Background()
+	err := c.Alter(ctx, &api.Operation{
+		Schema: fmt.Sprintf(`%s: uid @reverse .`, largePred),
+	})
+	require.Error(t, err)
+	require.Contains(t, err.Error(), "Predicate name length cannot be bigger than 2^16")
+
+	// Verify that Mutate requests with predicates that are too large are rejected.
+	txn := c.NewTxn()
+	_, err = txn.Mutate(ctx, &api.Mutation{
+		CommitNow: true,
+		SetNquads: []byte(fmt.Sprintf(`_:test <%s> "value" .`, largePred)),
+	})
+	require.Error(t, err)
+	require.Contains(t, err.Error(), "Predicate name length cannot be bigger than 2^16")
+	_ = txn.Discard(ctx)
+
+	// Do the same thing as above but for the predicates in DelNquads.
+	txn = c.NewTxn()
+	defer txn.Discard(ctx)
+	_, err = txn.Mutate(ctx, &api.Mutation{
+		CommitNow: true,
+		DelNquads: []byte(fmt.Sprintf(`_:test <%s> "value" .`, largePred)),
+	})
+	require.Error(t, err)
+	require.Contains(t, err.Error(), "Predicate name length cannot be bigger than 2^16")
+}