Skip to content

Commit 5bbca7b

Browse files
mtrezzamtrezza
authored
fix: LDAP injection via unsanitized user input in DN and group filter construction ([GHSA-7m6r-fhh7-r47c](GHSA-7m6r-fhh7-r47c)) (#10154)
1 parent e2687b0 commit 5bbca7b

File tree

2 files changed

+214
-3
lines changed

2 files changed

+214
-3
lines changed

‎spec/LdapAuth.spec.js‎

Lines changed: 172 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,178 @@ const fs = require('fs');
44
const port = 12345;
55
const sslport = 12346;
66

7+
describe('LDAP Injection Prevention', () => {
8+
describe('escapeDN', () => {
9+
it('should escape comma', () => {
10+
expect(ldap.escapeDN('admin,ou=evil')).toBe('admin\\,ou\\=evil');
11+
});
12+
13+
it('should escape equals sign', () => {
14+
expect(ldap.escapeDN('admin=evil')).toBe('admin\\=evil');
15+
});
16+
17+
it('should escape plus sign', () => {
18+
expect(ldap.escapeDN('admin+evil')).toBe('admin\\+evil');
19+
});
20+
21+
it('should escape less-than and greater-than signs', () => {
22+
expect(ldap.escapeDN('admin<evil>')).toBe('admin\\<evil\\>');
23+
});
24+
25+
it('should escape hash at start', () => {
26+
expect(ldap.escapeDN('#admin')).toBe('\\#admin');
27+
});
28+
29+
it('should escape semicolon', () => {
30+
expect(ldap.escapeDN('admin;evil')).toBe('admin\\;evil');
31+
});
32+
33+
it('should escape double quote', () => {
34+
expect(ldap.escapeDN('admin"evil')).toBe('admin\\"evil');
35+
});
36+
37+
it('should escape backslash', () => {
38+
expect(ldap.escapeDN('admin\\evil')).toBe('admin\\\\evil');
39+
});
40+
41+
it('should escape leading space', () => {
42+
expect(ldap.escapeDN(' admin')).toBe('\\ admin');
43+
});
44+
45+
it('should escape trailing space', () => {
46+
expect(ldap.escapeDN('admin ')).toBe('admin\\ ');
47+
});
48+
49+
it('should escape multiple special characters', () => {
50+
expect(ldap.escapeDN('admin,ou=evil+cn=x')).toBe('admin\\,ou\\=evil\\+cn\\=x');
51+
});
52+
53+
it('should not modify safe values', () => {
54+
expect(ldap.escapeDN('testuser')).toBe('testuser');
55+
expect(ldap.escapeDN('john.doe')).toBe('john.doe');
56+
expect(ldap.escapeDN('user123')).toBe('user123');
57+
});
58+
});
59+
60+
describe('escapeFilter', () => {
61+
it('should escape asterisk', () => {
62+
expect(ldap.escapeFilter('*')).toBe('\\2a');
63+
});
64+
65+
it('should escape open parenthesis', () => {
66+
expect(ldap.escapeFilter('test(')).toBe('test\\28');
67+
});
68+
69+
it('should escape close parenthesis', () => {
70+
expect(ldap.escapeFilter('test)')).toBe('test\\29');
71+
});
72+
73+
it('should escape backslash', () => {
74+
expect(ldap.escapeFilter('test\\')).toBe('test\\5c');
75+
});
76+
77+
it('should escape null byte', () => {
78+
expect(ldap.escapeFilter('test\x00')).toBe('test\\00');
79+
});
80+
81+
it('should escape multiple special characters', () => {
82+
expect(ldap.escapeFilter('*()\\')).toBe('\\2a\\28\\29\\5c');
83+
});
84+
85+
it('should not modify safe values', () => {
86+
expect(ldap.escapeFilter('testuser')).toBe('testuser');
87+
expect(ldap.escapeFilter('john.doe')).toBe('john.doe');
88+
expect(ldap.escapeFilter('user123')).toBe('user123');
89+
});
90+
91+
it('should escape filter injection attempt with wildcard', () => {
92+
expect(ldap.escapeFilter('x)(|(objectClass=*)')).toBe('x\\29\\28|\\28objectClass=\\2a\\29');
93+
});
94+
});
95+
96+
describe('authData validation', () => {
97+
it('should reject missing authData.id', async done => {
98+
const server = await mockLdapServer(port, 'uid=testuser, o=example');
99+
const options = {
100+
suffix: 'o=example',
101+
url: `ldap://localhost:${port}`,
102+
dn: 'uid={{id}}, o=example',
103+
};
104+
try {
105+
await ldap.validateAuthData({ password: 'secret' }, options);
106+
fail('Should have rejected missing id');
107+
} catch (err) {
108+
expect(err.message).toBe('LDAP: Wrong username or password');
109+
}
110+
server.close(done);
111+
});
112+
113+
it('should reject non-string authData.id', async done => {
114+
const server = await mockLdapServer(port, 'uid=testuser, o=example');
115+
const options = {
116+
suffix: 'o=example',
117+
url: `ldap://localhost:${port}`,
118+
dn: 'uid={{id}}, o=example',
119+
};
120+
try {
121+
await ldap.validateAuthData({ id: 123, password: 'secret' }, options);
122+
fail('Should have rejected non-string id');
123+
} catch (err) {
124+
expect(err.message).toBe('LDAP: Wrong username or password');
125+
}
126+
server.close(done);
127+
});
128+
});
129+
130+
describe('DN injection prevention', () => {
131+
it('should prevent DN injection via comma in authData.id', async done => {
132+
// Mock server accepts the DN that would result from an unescaped injection
133+
const server = await mockLdapServer(port, 'uid=admin,ou=admins,o=example');
134+
const options = {
135+
suffix: 'o=example',
136+
url: `ldap://localhost:${port}`,
137+
dn: 'uid={{id}}, o=example',
138+
};
139+
// Attacker tries to inject additional DN components via comma
140+
// Without escaping: DN = uid=admin,ou=admins, o=example (3 RDNs) → matches mock
141+
// With escaping: DN = uid=admin\,ou=admins, o=example (2 RDNs) → doesn't match
142+
try {
143+
await ldap.validateAuthData({ id: 'admin,ou=admins', password: 'secret' }, options);
144+
fail('Should have rejected DN injection attempt');
145+
} catch (err) {
146+
expect(err.message).toBe('LDAP: Wrong username or password');
147+
}
148+
server.close(done);
149+
});
150+
});
151+
152+
describe('Filter injection prevention', () => {
153+
it('should prevent LDAP filter injection via wildcard in authData.id', async done => {
154+
// Mock server accepts uid=*, o=example (the attacker's bind DN)
155+
// The * is not special in DNs so it binds fine regardless of escaping
156+
const server = await mockLdapServer(port, 'uid=*, o=example');
157+
const options = {
158+
suffix: 'o=example',
159+
url: `ldap://localhost:${port}`,
160+
dn: 'uid={{id}}, o=example',
161+
groupCn: 'powerusers',
162+
groupFilter: '(&(uniqueMember=uid={{id}}, o=example)(objectClass=groupOfUniqueNames))',
163+
};
164+
// Attacker uses * as ID to match any group member via wildcard
165+
// Group has member uid=testuser, not uid=*
166+
// Without escaping: filter uses SubstringFilter, matches testuser → passes
167+
// With escaping: filter uses EqualityFilter with literal \2a, no match → fails
168+
try {
169+
await ldap.validateAuthData({ id: '*', password: 'secret' }, options);
170+
fail('Should have rejected filter injection attempt');
171+
} catch (err) {
172+
expect(err.message).toBe('LDAP: User not in group');
173+
}
174+
server.close(done);
175+
});
176+
});
177+
});
178+
7179
describe('Ldap Auth', () => {
8180
it('Should fail with missing options', done => {
9181
ldap

‎src/Adapters/Auth/ldap.js‎

Lines changed: 42 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -76,6 +76,37 @@
7676
const ldapjs = require('ldapjs');
7777
const Parse = require('parse/node').Parse;
7878

79+
// Escape LDAP DN special characters per RFC 4514
80+
// https://datatracker.ietf.org/doc/html/rfc4514#section-2.4
81+
function escapeDN(value) {
82+
let escaped = value
83+
.replace(/\\/g, '\\\\')
84+
.replace(/,/g, '\\,')
85+
.replace(/=/g, '\\=')
86+
.replace(/\+/g, '\\+')
87+
.replace(/</g, '\\<')
88+
.replace(/>/g, '\\>')
89+
.replace(/#/g, '\\#')
90+
.replace(/;/g, '\\;')
91+
.replace(/"/g, '\\"');
92+
if (escaped.startsWith(' ')) {
93+
escaped = '\\ ' + escaped.slice(1);
94+
}
95+
if (escaped.endsWith(' ')) {
96+
escaped = escaped.slice(0, -1) + '\\ ';
97+
}
98+
return escaped;
99+
}
100+
101+
// Escape LDAP filter special characters per RFC 4515
102+
// https://datatracker.ietf.org/doc/html/rfc4515#section-3
103+
function escapeFilter(value) {
104+
// eslint-disable-next-line no-control-regex
105+
return value.replace(/[\\*()\x00]/g, ch =>
106+
'\\' + ch.charCodeAt(0).toString(16).padStart(2, '0')
107+
);
108+
}
109+
79110
function validateAuthData(authData, options) {
80111
if (!optionsAreValid(options)) {
81112
return new Promise((_, reject) => {
@@ -86,11 +117,17 @@ function validateAuthData(authData, options) {
86117
? { url: options.url, tlsOptions: options.tlsOptions }
87118
: { url: options.url };
88119

120+
if (typeof authData.id !== 'string') {
121+
return Promise.reject(
122+
new Parse.Error(Parse.Error.OBJECT_NOT_FOUND, 'LDAP: Wrong username or password')
123+
);
124+
}
89125
const client = ldapjs.createClient(clientOptions);
126+
const escapedId = escapeDN(authData.id);
90127
const userCn =
91128
typeof options.dn === 'string'
92-
? options.dn.replace('{{id}}', authData.id)
93-
: `uid=${authData.id},${options.suffix}`;
129+
? options.dn.replace('{{id}}', escapedId)
130+
: `uid=${escapedId},${options.suffix}`;
94131

95132
return new Promise((resolve, reject) => {
96133
client.bind(userCn, authData.password, ldapError => {
@@ -140,7 +177,7 @@ function optionsAreValid(options) {
140177
}
141178

142179
function searchForGroup(client, options, id, resolve, reject) {
143-
const filter = options.groupFilter.replace(/{{id}}/gi, id);
180+
const filter = options.groupFilter.replace(/{{id}}/gi, escapeFilter(id));
144181
const opts = {
145182
scope: 'sub',
146183
filter: filter,
@@ -184,4 +221,6 @@ function validateAppId() {
184221
module.exports = {
185222
validateAppId,
186223
validateAuthData,
224+
escapeDN,
225+
escapeFilter,
187226
};

0 commit comments

Comments
 (0)
⚡