From efb49049104196bae311e15d131498c1cd78ee99 Mon Sep 17 00:00:00 2001 From: Michael Aldridge Date: Sun, 1 Nov 2026 18:55:53 -0600 Subject: [PATCH] internal/ldap: Quote search values to prevent query injection --- internal/ldap/bleve_mapper.go | 17 +++++- internal/ldap/bleve_mapper_test.go | 85 ++++++++++++++++++++++++++++++ internal/ldap/testutil_test.go | 46 ++++++++++++++++ 3 files changed, 147 insertions(+), 1 deletion(-) create mode 100644 internal/ldap/bleve_mapper_test.go diff --git a/internal/ldap/bleve_mapper.go b/internal/ldap/bleve_mapper.go index 6e4d680..a72d0fb 100644 --- a/internal/ldap/bleve_mapper.go +++ b/internal/ldap/bleve_mapper.go @@ -54,5 +54,20 @@ func mapToBleveStringQuery(attr, op, val string) (string, error) { return "", errors.New("search comparison is unsupported") } - return predicate + operator + val, nil + return predicate + operator + quoteBleveValue(val), nil +} + +// quoteBleveValue makes a value safe to embed in a Bleve query string. +// +// Assertion values come straight from the network and Bleve parses its +// queries, so an unquoted value such as "x Name:y" would add terms the +// client never asked for. Quoting keeps the value as a single term, and +// the wildcard used for attribute-present filters stays outside the +// quotes so that it keeps its meaning. +func quoteBleveValue(val string) string { + if val == "*" { + return val + } + escaped := strings.NewReplacer(`\`, `\\`, `"`, `\"`).Replace(val) + return `"` + escaped + `"` } diff --git a/internal/ldap/bleve_mapper_test.go b/internal/ldap/bleve_mapper_test.go new file mode 100644 index 0000000..ef92ab5 --- /dev/null +++ b/internal/ldap/bleve_mapper_test.go @@ -0,0 +1,85 @@ +package ldap + +import ( + "testing" + + "github.com/ps78674/goldap/message" +) + +func TestBuildBleveQuery(t *testing.T) { + s := &server{l: testLogger()} + + cases := []struct { + name string + filter message.Filter + want string + }{ + { + name: "equality on uid", + filter: berEqualityFilter(t, "uid", "jsmith"), + want: `ID:"jsmith"`, + }, + { + name: "equality on cn", + filter: berEqualityFilter(t, "cn", "admins"), + want: `Name:"admins"`, + }, + { + name: "attribute present", + filter: berPresentFilter(t, "uid"), + want: "ID:*", + }, + { + name: "or of two matches", + filter: berOrFilter(t, + berRawEquality("uid", "jsmith"), + berRawEquality("uid", "jdoe"), + ), + want: `ID:"jsmith" ID:"jdoe"`, + }, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + got, err := s.buildBleveQuery(tc.filter) + if err != nil { + t.Fatalf("unexpected error: %s", err) + } + if got != tc.want { + t.Errorf("got %q, want %q", got, tc.want) + } + }) + } +} + +// Assertion values are parsed by Bleve, so they must not be able to +// introduce extra query terms or operators. +func TestBuildBleveQueryEscapesValues(t *testing.T) { + s := &server{l: testLogger()} + + cases := []struct { + value string + want string + }{ + {"x Name:y", `ID:"x Name:y"`}, + {`x" OR ID:"*`, `ID:"x\" OR ID:\"*"`}, + {`back\slash`, `ID:"back\\slash"`}, + {"x AND Name:admin", `ID:"x AND Name:admin"`}, + } + for _, tc := range cases { + got, err := s.buildBleveQuery(berEqualityFilter(t, "uid", tc.value)) + if err != nil { + t.Fatalf("%q: unexpected error: %s", tc.value, err) + } + if got != tc.want { + t.Errorf("value %q: got %q, want %q", tc.value, got, tc.want) + } + } +} + +func TestBuildBleveQueryUnsupported(t *testing.T) { + s := &server{l: testLogger()} + + if _, err := s.buildBleveQuery(berPresentFilter(t, "mail")); err == nil { + t.Error("an unsupported attribute should not produce a query") + } +} diff --git a/internal/ldap/testutil_test.go b/internal/ldap/testutil_test.go index 2c2251d..ab291dc 100644 --- a/internal/ldap/testutil_test.go +++ b/internal/ldap/testutil_test.go @@ -55,6 +55,52 @@ func berSearchRequest(baseDN string) []byte { return berElement(0x30, berInteger(2), search) } +// berEqualityFilter encodes an equalityMatch [3] +// AttributeValueAssertion filter and decodes it through the real BER +// reader. The filter types have unexported fields and no exported +// constructors, so this is the only way to build one for a test. +func berEqualityFilter(t *testing.T, attr, value string) message.Filter { + t.Helper() + return berDecodeFilter(t, berElement(0xa3, berOctetString(attr), berOctetString(value))) +} + +// berPresentFilter encodes a present [7] AttributeDescription filter. +func berPresentFilter(t *testing.T, attr string) message.Filter { + t.Helper() + return berDecodeFilter(t, berElement(0x87, []byte(attr))) +} + +// berOrFilter encodes an OR [1] of already-encoded filter elements. +func berOrFilter(t *testing.T, filters ...[]byte) message.Filter { + t.Helper() + return berDecodeFilter(t, berElement(0xa1, filters...)) +} + +// berRawEquality returns the encoded bytes of an equality match so it +// can be composed into a nested filter. +func berRawEquality(attr, value string) []byte { + return berElement(0xa3, berOctetString(attr), berOctetString(value)) +} + +// berDecodeFilter wraps a raw filter element in a minimal search +// request, decodes it and returns the parsed filter. +func berDecodeFilter(t *testing.T, filterElement []byte) message.Filter { + t.Helper() + search := berElement(0x63, + berOctetString(""), // baseObject + berElement(0x0a, []byte{2}), // scope wholeSubtree + berElement(0x0a, []byte{0}), // derefAliases never + berInteger(0), // sizeLimit + berInteger(0), // timeLimit + berElement(0x01, []byte{0}), // typesOnly false + filterElement, // filter + berElement(0x30), // empty attribute selection + ) + m := mustDecode(t, berElement(0x30, berInteger(1), search)) + r := m.GetSearchRequest() + return r.Filter() +} + func mustDecode(t *testing.T, raw []byte) *ldap.Message { t.Helper() m, err := message.ReadLDAPMessage(message.NewBytes(0, raw))