Skip to content

Commit 35356e0

Browse files
t2yt2y
authored
fix: reject unescaped special characters in DN values per RFC 4514 (#588)
ParseDN previously accepted ", ;, <, > and NULL (U+0000) in raw form, silently producing values that violate the RFC 4514 grammar and could be misinterpreted by downstream LDAP servers or string-comparison code. decodeString now returns an error when it encounters these characters unescaped; their hex- and backslash-escaped forms continue to decode as before. ParseDN tests are expanded to cover every RFC 4514 escape target in both escape forms, plus the corresponding unescaped-error cases. Leading/trailing U+0020 (SPACE) is intentionally left lenient — the existing whitespace-stripping behavior is depended on by many real inputs, so enforcing it strictly is deferred.
1 parent bf231eb commit 35356e0

2 files changed

Lines changed: 131 additions & 5 deletions

File tree

‎v3/dn.go‎

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,11 +4,12 @@ import (
44
"encoding/hex"
55
"errors"
66
"fmt"
7-
ber "github.com/go-asn1-ber/asn1-ber"
87
"sort"
98
"strings"
109
"unicode"
1110
"unicode/utf8"
11+
12+
ber "github.com/go-asn1-ber/asn1-ber"
1213
)
1314

1415
// AttributeTypeAndValue represents an attributeTypeAndValue from https://tools.ietf.org/html/rfc4514
@@ -116,6 +117,16 @@ func decodeString(str string) (string, error) {
116117
// If the character is not an escape character, just add it to the
117118
// builder and continue
118119
if char != '\\' {
120+
// RFC 4514 section 2.4: these characters must appear escaped
121+
// (either as "\X" or as "\XX" hex) when present in an AttributeValue.
122+
// Reject the raw form here so that callers don't silently accept
123+
// input that violates the grammar.
124+
switch char {
125+
case '"', ';', '<', '>':
126+
return "", fmt.Errorf("got unescaped character: '%s'", string(char))
127+
case 0:
128+
return "", fmt.Errorf("got unescaped NULL character")
129+
}
119130
builder.WriteRune(char)
120131
continue
121132
}

‎v3/dn_test.go‎

Lines changed: 119 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -116,6 +116,86 @@ func TestSuccessfulDNParsing(t *testing.T) {
116116
{[]*AttributeTypeAndValue{{"ou", "Baz"}}},
117117
{[]*AttributeTypeAndValue{{"o", "C; orp."}, {"c", "US"}}},
118118
}},
119+
120+
// RFC 4514 section 2.4: characters that require escaping in an AttributeValue.
121+
// Each character is exercised with both the backslash-escape form (e.g. "\,")
122+
// and the hex-escape form (e.g. "\2C"), except for the NULL character (U+0000)
123+
// which only has the hex form.
124+
125+
// U+0020 (SPACE) — only requires escaping at the beginning or end of a value.
126+
`cn=\ John Doe`: {[]*RelativeDN{
127+
{[]*AttributeTypeAndValue{{"cn", " John Doe"}}},
128+
}},
129+
`cn=John Doe\ `: {[]*RelativeDN{
130+
{[]*AttributeTypeAndValue{{"cn", "John Doe "}}},
131+
}},
132+
`cn=\20John Doe`: {[]*RelativeDN{
133+
{[]*AttributeTypeAndValue{{"cn", " John Doe"}}},
134+
}},
135+
`cn=John Doe\20`: {[]*RelativeDN{
136+
{[]*AttributeTypeAndValue{{"cn", "John Doe "}}},
137+
}},
138+
139+
// U+0022 (QUOTATION MARK)
140+
`cn=John \"Doe\"`: {[]*RelativeDN{
141+
{[]*AttributeTypeAndValue{{"cn", `John "Doe"`}}},
142+
}},
143+
`cn=John \22Doe\22`: {[]*RelativeDN{
144+
{[]*AttributeTypeAndValue{{"cn", `John "Doe"`}}},
145+
}},
146+
147+
// U+002B (PLUS SIGN)
148+
`cn=John\+Doe`: {[]*RelativeDN{
149+
{[]*AttributeTypeAndValue{{"cn", "John+Doe"}}},
150+
}},
151+
`cn=John\2BDoe`: {[]*RelativeDN{
152+
{[]*AttributeTypeAndValue{{"cn", "John+Doe"}}},
153+
}},
154+
155+
// U+002C (COMMA)
156+
`cn=Doe\, John`: {[]*RelativeDN{
157+
{[]*AttributeTypeAndValue{{"cn", "Doe, John"}}},
158+
}},
159+
`cn=Doe\2C John`: {[]*RelativeDN{
160+
{[]*AttributeTypeAndValue{{"cn", "Doe, John"}}},
161+
}},
162+
163+
// U+003B (SEMICOLON)
164+
`cn=John\;Doe`: {[]*RelativeDN{
165+
{[]*AttributeTypeAndValue{{"cn", "John;Doe"}}},
166+
}},
167+
`cn=John\3BDoe`: {[]*RelativeDN{
168+
{[]*AttributeTypeAndValue{{"cn", "John;Doe"}}},
169+
}},
170+
171+
// U+003C (LESS-THAN SIGN)
172+
`cn=John\<Doe`: {[]*RelativeDN{
173+
{[]*AttributeTypeAndValue{{"cn", "John<Doe"}}},
174+
}},
175+
`cn=John\3CDoe`: {[]*RelativeDN{
176+
{[]*AttributeTypeAndValue{{"cn", "John<Doe"}}},
177+
}},
178+
179+
// U+003E (GREATER-THAN SIGN)
180+
`cn=John\>Doe`: {[]*RelativeDN{
181+
{[]*AttributeTypeAndValue{{"cn", "John>Doe"}}},
182+
}},
183+
`cn=John\3EDoe`: {[]*RelativeDN{
184+
{[]*AttributeTypeAndValue{{"cn", "John>Doe"}}},
185+
}},
186+
187+
// U+005C (REVERSE SOLIDUS / BACKSLASH)
188+
`cn=John\\Doe`: {[]*RelativeDN{
189+
{[]*AttributeTypeAndValue{{"cn", `John\Doe`}}},
190+
}},
191+
`cn=John\5CDoe`: {[]*RelativeDN{
192+
{[]*AttributeTypeAndValue{{"cn", `John\Doe`}}},
193+
}},
194+
195+
// U+0000 (NULL) — RFC 4514 only allows the hex-escape form.
196+
`cn=John\00Doe`: {[]*RelativeDN{
197+
{[]*AttributeTypeAndValue{{"cn", "John\x00Doe"}}},
198+
}},
119199
}
120200

121201
for test, answer := range testcases {
@@ -155,12 +235,47 @@ func TestErrorDNParsing(t *testing.T) {
155235
`1.3.6.1.4.1.1466.0=test;`: "DN ended with incomplete type, value pair",
156236
"1.3.6.1.4.1.1466.0=test+,": "incomplete type, value pair",
157237
"DF=#6666666666665006838820013100000746939546349182108463491821809FBFFFFFFFFF": "failed to decode BER encoding: unexpected EOF",
238+
239+
// RFC 4514 section 2.4: each of the following characters must be
240+
// escaped when they appear in an AttributeValue. The cases below place
241+
// the character in a position the spec disallows and expect a parse
242+
// error. Some tests may currently fail because the parser does not yet
243+
// reject every form of unescaped special character.
244+
245+
// U+0020 (SPACE) — leading/trailing space in a value must be escaped.
246+
// however, the blow is ignored and trimmed for compatibility.
247+
// ` cn=John,dc=com` => 'cn=John,dc=com'
248+
// `cn=John ,dc=com` => 'cn=John,dc=com'
249+
250+
// U+0022 (QUOTATION MARK)
251+
`cn=John "Doe",dc=com`: "got unescaped character: '\"'",
252+
253+
// U+002B (PLUS SIGN) — unescaped '+' acts as an RDN attribute separator.
254+
"cn=John+": "DN ended with incomplete type, value pair",
255+
256+
// U+002C (COMMA) — unescaped ',' acts as an RDN separator.
257+
"cn=John,": "DN ended with incomplete type, value pair",
258+
259+
// U+003B (SEMICOLON) — unescaped ';' acts as an RDN separator.
260+
"cn=John;": "DN ended with incomplete type, value pair",
261+
262+
// U+003C (LESS-THAN SIGN)
263+
"cn=John<Doe,dc=com": "got unescaped character: '<'",
264+
265+
// U+003E (GREATER-THAN SIGN)
266+
"cn=John>Doe,dc=com": "got unescaped character: '>'",
267+
268+
// U+005C (REVERSE SOLIDUS) — a lone trailing backslash is a corrupted escape.
269+
`cn=John\`: `got corrupted escaped character: 'John\'`,
270+
271+
// U+0000 (NULL)
272+
"cn=John\x00Doe,dc=com": "got unescaped NULL character",
158273
}
159274

160275
for test, answer := range testcases {
161-
_, err := ParseDN(test)
276+
dn, err := ParseDN(test)
162277
if err == nil {
163-
t.Errorf("Expected '%s' to fail parsing but succeeded\n", test)
278+
t.Errorf("Expected '%s' to fail parsing but succeeded as '%s'\n", test, dn.String())
164279
} else if err.Error() != answer {
165280
t.Errorf("Unexpected error on: '%s':\nExpected: %s\nGot: %s\n", test, answer, err.Error())
166281
}
@@ -370,7 +485,7 @@ func TestRoundTripLiteralSubject(t *testing.T) {
370485
rdnSequences := map[string]string{
371486
"cn=foo-long.com,ou=FooLong,ou=Barq,ou=Baz,ou=Dept.,o=Corp.,c=US": "cn=foo-long.com,ou=FooLong,ou=Barq,ou=Baz,ou=Dept.,o=Corp.,c=US",
372487
"cn=foo-lon❤️\\,g.com,ou=Foo===Long,ou=Ba # rq,ou=Baz,o=C\\; orp.,c=US": "cn=foo-lon\\e2\\9d\\a4\\ef\\b8\\8f\\,g.com,ou=Foo===Long,ou=Ba # rq,ou=Baz,o=C\\; orp.,c=US",
373-
"cn=fo\x00o-long.com,ou=\x04FooLong": "cn=fo\\00o-long.com,ou=\\04FooLong",
488+
"cn=fo\\00o-long.com,ou=\x04FooLong": "cn=fo\\00o-long.com,ou=\\04FooLong",
374489
}
375490

376491
for subjIn, subjOut := range rdnSequences {
@@ -389,7 +504,6 @@ func TestDecodeString(t *testing.T) {
389504
successTestcases := map[string]string{
390505
"foo-long.com": "foo-long.com",
391506
"foo-lon❤️\\,g.com": "foo-lon❤️,g.com",
392-
"fo\x00o-long.com": "fo\x00o-long.com",
393507
"fo\\00o-long.com": "fo\x00o-long.com",
394508
}
395509

@@ -408,6 +522,7 @@ func TestDecodeString(t *testing.T) {
408522
"fo\\0": "failed to decode escaped character: encoding/hex: invalid byte: 0",
409523
"fo\\UU️o-long.com": "failed to decode escaped character: encoding/hex: invalid byte: U+0055 'U'",
410524
"fo\\0❤️o-long.com": "failed to decode escaped character: invalid byte: 0❤",
525+
"fo\x00o-long.com": "got unescaped NULL character",
411526
}
412527

413528
for encoded, expectedError := range errorTestcases {

0 commit comments

Comments
 (0)