Sanitize cypher errors from neo4j driver (#526) Fixes #278

* chore: sanitize cypher queries in driver errors

* test: add unit test for stripCypherQuery

* fix: IsNeoTimeoutError should handle graph.Error properly

packages/go/dawgs/drivers/neo4j/cypher.go

@@ -17,10 +17,14 @@
 package neo4j
 
 import (
+	"bytes"
 	"sort"
 	"strings"
 
+	"github.com/specterops/bloodhound/cypher/backend/cypher"
+	"github.com/specterops/bloodhound/cypher/frontend"
 	"github.com/specterops/bloodhound/dawgs/graph"
+	"github.com/specterops/bloodhound/log"
 )
 
 func updateKey(identityKind graph.Kind, identityProperties []string, updateKinds graph.Kinds) string {
@@ -283,3 +287,18 @@ func cypherBuildNodeUpdateQueryBatch(updates []graph.NodeUpdate) ([]string, []ma
 
 	return queries, queryParameters
 }
+
+func stripCypherQuery(rawQuery string) string {
+	var (
+		strippedEmitter = cypher.NewCypherEmitter(true)
+		buffer          = &bytes.Buffer{}
+	)
+
+	if queryModel, err := frontend.ParseCypher(frontend.DefaultCypherContext(), rawQuery); err != nil {
+		log.Errorf("error occurred parsing cypher query during sanitization: %v", err)
+	} else if err = strippedEmitter.Write(queryModel, buffer); err != nil {
+		log.Errorf("error occurred sanitizing cypher query: %v", err)
+	}
+
+	return buffer.String()
+}

packages/go/dawgs/drivers/neo4j/cypher_internal_test.go

@@ -0,0 +1,19 @@
+package neo4j
+
+import (
+	"strings"
+	"testing"
+
+	"github.com/stretchr/testify/require"
+)
+
+func Test_StripCypher(t *testing.T) {
+	var (
+		query = "match (u1:User {domain: \"DOMAIN1\"}), (u2:User {domain: \"DOMAIN2\"}) where u1.samaccountname <> \"krbtgt\" and u1.samaccountname = u2.samaccountname with u2 match p1 = (u2)-[*1..]->(g:Group) with p1 match p2 = (u2)-[*1..]->(g:Group) return p1, p2"
+	)
+
+	result := stripCypherQuery(query)
+
+	require.Equalf(t, false, strings.Contains(result, "DOMAIN1"), "Cypher query not sanitized. Contains sensitive value: %s", result)
+	require.Equalf(t, false, strings.Contains(result, "DOMAIN2"), "Cypher query not sanitized. Contains sensitive value: %s", result)
+}

packages/go/dawgs/drivers/neo4j/node.go

@@ -160,7 +160,8 @@ func (s *NodeQuery) Update(properties *graph.Properties) error {
 	if err := s.queryBuilder.Prepare(); err != nil {
 		return err
 	} else if cypherQuery, err := s.queryBuilder.Render(); err != nil {
-		return graph.NewError(cypherQuery, err)
+		strippedQuery := stripCypherQuery(cypherQuery)
+		return graph.NewError(strippedQuery, err)
 	} else if result := s.run(cypherQuery, s.queryBuilder.Parameters); result.Error() != nil {
 		return result.Error()
 	}

packages/go/dawgs/drivers/neo4j/relationship.go

@@ -121,7 +121,8 @@ func (s *RelationshipQuery) Update(properties *graph.Properties) error {
 	if err := s.queryBuilder.Prepare(); err != nil {
 		return err
 	} else if cypherQuery, err := s.queryBuilder.Render(); err != nil {
-		return graph.NewError(cypherQuery, err)
+		strippedQuery := stripCypherQuery(cypherQuery)
+		return graph.NewError(strippedQuery, err)
 	} else {
 		return s.run(cypherQuery, s.queryBuilder.Parameters).Error()
 	}

packages/go/dawgs/drivers/neo4j/result.go

@@ -57,7 +57,8 @@ func (s *internalResult) Error() error {
 	}
 
 	if s.driverResult != nil && s.driverResult.Err() != nil {
-		return graph.NewError(s.query, s.driverResult.Err())
+		strippedQuery := stripCypherQuery(s.query)
+		return graph.NewError(strippedQuery, s.driverResult.Err())
 	}
 
 	return nil

packages/go/dawgs/drivers/neo4j/transaction.go

@@ -20,11 +20,12 @@ import (
 	"context"
 	"encoding/json"
 	"fmt"
-	"github.com/specterops/bloodhound/dawgs/drivers"
-	"github.com/specterops/bloodhound/log"
 	"sort"
 	"strings"
 
+	"github.com/specterops/bloodhound/dawgs/drivers"
+	"github.com/specterops/bloodhound/log"
+
 	"github.com/specterops/bloodhound/dawgs/query/neo4j"
 	"github.com/specterops/bloodhound/dawgs/util/size"
 
@@ -226,7 +227,8 @@ func (s *neo4jTransaction) updateNode(updatedNode *graph.Node) error {
 	if err := queryBuilder.Prepare(); err != nil {
 		return err
 	} else if cypherQuery, err := queryBuilder.Render(); err != nil {
-		return graph.NewError(cypherQuery, err)
+		strippedQuery := stripCypherQuery(cypherQuery)
+		return graph.NewError(strippedQuery, err)
 	} else if result := s.Raw(cypherQuery, queryBuilder.Parameters); result.Error() != nil {
 		return result.Error()
 	}
@@ -454,7 +456,8 @@ func (s *neo4jTransaction) UpdateRelationship(relationship *graph.Relationship)
 	if err := queryBuilder.Prepare(); err != nil {
 		return err
 	} else if cypherQuery, err := queryBuilder.Render(); err != nil {
-		return graph.NewError(cypherQuery, err)
+		strippedQuery := stripCypherQuery(cypherQuery)
+		return graph.NewError(strippedQuery, err)
 	} else {
 		return s.runAndLog(cypherQuery, queryBuilder.Parameters, 1).Error()
 	}

packages/go/dawgs/util/errors.go

@@ -1,26 +1,28 @@
 // Copyright 2023 Specter Ops, Inc.
-// 
+//
 // Licensed under the Apache License, Version 2.0
 // you may not use this file except in compliance with the License.
 // You may obtain a copy of the License at
-// 
+//
 //     http://www.apache.org/licenses/LICENSE-2.0
-// 
+//
 // Unless required by applicable law or agreed to in writing, software
 // distributed under the License is distributed on an "AS IS" BASIS,
 // WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
 // See the License for the specific language governing permissions and
 // limitations under the License.
-// 
+//
 // SPDX-License-Identifier: Apache-2.0
 
 package util
 
 import (
 	"errors"
-	"github.com/neo4j/neo4j-go-driver/v5/neo4j"
 	"strings"
 	"sync"
+
+	"github.com/neo4j/neo4j-go-driver/v5/neo4j"
+	"github.com/specterops/bloodhound/dawgs/graph"
 )
 
 type ErrorCollector interface {
@@ -58,9 +60,14 @@ func (s *errorCollector) Combined() error {
 }
 
 func IsNeoTimeoutError(err error) bool {
-	if castError, ok := err.(*neo4j.Neo4jError); !ok {
+	switch e := err.(type) {
+	case *neo4j.Neo4jError:
+		return strings.Contains(e.Code, "TransactionTimedOut")
+	case graph.Error:
+		return strings.Contains(e.Error(), "Neo.ClientError.Transaction.TransactionTimedOut")
+	case *graph.Error:
+		return strings.Contains(e.Error(), "Neo.ClientError.Transaction.TransactionTimedOut")
+	default:
 		return false
-	} else {
-		return strings.Contains(castError.Code, "TransactionTimedOut")
 	}
 }

packages/go/dawgs/util/errors_test.go

@@ -1,41 +1,53 @@
 // Copyright 2023 Specter Ops, Inc.
-// 
+//
 // Licensed under the Apache License, Version 2.0
 // you may not use this file except in compliance with the License.
 // You may obtain a copy of the License at
-// 
+//
 //     http://www.apache.org/licenses/LICENSE-2.0
-// 
+//
 // Unless required by applicable law or agreed to in writing, software
 // distributed under the License is distributed on an "AS IS" BASIS,
 // WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
 // See the License for the specific language governing permissions and
 // limitations under the License.
-// 
+//
 // SPDX-License-Identifier: Apache-2.0
 
 package util_test
 
 import (
+	"errors"
 	"testing"
 
 	"github.com/neo4j/neo4j-go-driver/v5/neo4j"
+	"github.com/specterops/bloodhound/dawgs/graph"
 	"github.com/specterops/bloodhound/dawgs/util"
 	"github.com/stretchr/testify/require"
 )
 
 func TestIsNeoTimeoutError(t *testing.T) {
-	err := neo4j.Neo4jError{
+	neoTimeOutErr := neo4j.Neo4jError{
 		Code: "Neo.ClientError.Transaction.TransactionTimedOut",
 		Msg:  "The transaction has been terminated. Retry your operation in a new transaction, and you should see a successful result. The transaction has not completed within the specified timeout (dbms.transaction.timeout). You may want to retry with a longer timeout.",
 	}
+	require.True(t, util.IsNeoTimeoutError(&neoTimeOutErr))
 
-	require.True(t, util.IsNeoTimeoutError(&err))
-
-	err = neo4j.Neo4jError{
+	notTimeOutErr := neo4j.Neo4jError{
 		Code: "This.Is.A.Test",
 		Msg:  "Blah",
 	}
+	require.False(t, util.IsNeoTimeoutError(&notTimeOutErr))
+
+	driverTimeOutErr := graph.Error{
+		Query:       "match (u1:User {domain: \"ESC6.LOCAL\"}), (u2:User {domain: \"ESC3.LOCAL\"}) where u1.samaccountname <> \"krbtgt\" and u1.samaccountname = u2.samaccountname with u2 match p1 = (u2)-[*1..]->(g:Group) with p1 match p2 = (u2)-[*1..]->(g:Group) return p1, p2",
+		DriverError: errors.New("Neo4jError: Neo.ClientError.Transaction.TransactionTimedOut (The transaction has been terminated. Retry your operation in a new transaction, and you should see a successful result. The transaction has not completed within the specified timeout (dbms.transaction.timeout). You may want to retry with a longer timeout. )"),
+	}
+	require.True(t, util.IsNeoTimeoutError(driverTimeOutErr))
 
-	require.False(t, util.IsNeoTimeoutError(&err))
+	notDriverTimeOutErr := graph.Error{
+		Query:       "match (u1:User {domain: \"ESC6.LOCAL\"}), (u2:User {domain: \"ESC3.LOCAL\"}) where u1.samaccountname <> \"krbtgt\" and u1.samaccountname = u2.samaccountname with u2 match p1 = (u2)-[*1..]->(g:Group) with p1 match p2 = (u2)-[*1..]->(g:Group) return p1, p2",
+		DriverError: errors.New("Some other error"),
+	}
+	require.False(t, util.IsNeoTimeoutError(notDriverTimeOutErr))
 }