From f32effe4430893eefbee78bd60015b6459497511 Mon Sep 17 00:00:00 2001 From: Kartik Kenchi Date: Tue, 13 Oct 2026 22:04:30 +0530 Subject: [PATCH] fix: stop EscapeDN and PostalAddress.Escape corrupting invalid UTF-8 --- v3/ldap.go | 14 ++++++++++++++ v3/ldap_test.go | 3 +++ v3/postaladdress.go | 11 ++++++++--- v3/postaladdress_test.go | 13 +++++++++++++ 4 files changed, 38 insertions(+), 3 deletions(-) diff --git a/v3/ldap.go b/v3/ldap.go index ab608c99..0ca11092 100644 --- a/v3/ldap.go +++ b/v3/ldap.go @@ -1,10 +1,12 @@ package ldap import ( + "encoding/hex" "fmt" "log" "os" "strings" + "unicode/utf8" ber "github.com/go-asn1-ber/asn1-ber" ) @@ -489,6 +491,18 @@ func EscapeDN(dn string) string { builder := strings.Builder{} for i, r := range dn { + // A byte that is not part of a valid UTF-8 sequence ranges as + // (utf8.RuneError, width 1). Writing the rune would silently replace + // the byte with U+FFFD, so two different values could escape to the + // same DN; escape the raw byte as an RFC 4514 hex pair instead. + if r == utf8.RuneError { + if _, size := utf8.DecodeRuneInString(dn[i:]); size == 1 { + builder.WriteByte('\\') + builder.WriteString(hex.EncodeToString([]byte{dn[i]})) + continue + } + } + // Escape leading and trailing spaces if (i == 0 || i == len(dn)-1) && r == ' ' { builder.WriteRune('\\') diff --git a/v3/ldap_test.go b/v3/ldap_test.go index fa4390f5..df5c330d 100644 --- a/v3/ldap_test.go +++ b/v3/ldap_test.go @@ -348,6 +348,9 @@ func TestEscapeDN(t *testing.T) { {name: "nullByte", dn: "\u0000te\x00st\x00user" + string(rune(0)), want: "\\00te\\00st\\00user\\00"}, {name: "variousCharacters", dn: "test\"+,;<>\\-_user", want: "test\\\"\\+\\,\\;\\<\\>\\\\-_user"}, {name: "multiByteRunes", dn: "test\u0391user ", want: "test\u0391user\\ "}, + {name: "invalidUTF8Byte", dn: "test\xebuser", want: "test\\ebuser"}, + {name: "truncatedUTF8Sequence", dn: "test\xc3", want: "test\\c3"}, + {name: "replacementCharKeptDistinct", dn: "test\ufffduser", want: "test\ufffduser"}, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { diff --git a/v3/postaladdress.go b/v3/postaladdress.go index 978478b5..302d801d 100644 --- a/v3/postaladdress.go +++ b/v3/postaladdress.go @@ -49,14 +49,19 @@ func (p *PostalAddress) Escape() string { builder := &strings.Builder{} for _, line := range p.lines { - for _, char := range line { - switch char { + // Iterate bytes, not runes: ranging a line that is not valid UTF-8 + // would replace each invalid byte with U+FFFD, so two different lines + // could escape to the same value. The escaped characters are ASCII and + // ParsePostalAddress reads bytes, so every other octet passes through + // unchanged. + for i := 0; i < len(line); i++ { + switch line[i] { case '\\': builder.WriteString("\\5C") case '$': builder.WriteString("\\24") default: - builder.WriteRune(char) + builder.WriteByte(line[i]) } } diff --git a/v3/postaladdress_test.go b/v3/postaladdress_test.go index 140e0610..21c83f0d 100644 --- a/v3/postaladdress_test.go +++ b/v3/postaladdress_test.go @@ -200,6 +200,19 @@ func TestPostalAddressEqual(t *testing.T) { } } +func TestPostalAddressEscapeInvalidUTF8(t *testing.T) { + // A line that is not valid UTF-8 must keep its original bytes when + // escaped; replacing them with U+FFFD would make two different lines + // escape to the same value. + addr := mustNewPostalAddress(t, []string{"caf\xe9 $1"}) + escaped := addr.Escape() + assert.Equal(t, "caf\xe9 \\241$", escaped) + + parsed, err := ParsePostalAddress(escaped) + assert.NoError(t, err) + assert.True(t, addr.Equal(parsed), "escaped form should parse back to the same lines") +} + func TestParsePostalAddress_Escape(t *testing.T) { t.Run("incomplete escape", func(t *testing.T) { _, err := ParsePostalAddress("AAAAAAAAAA\\")