Skip to content

Commit 3535e2f

Browse files
mtrezzamtrezza
committed
fix
1 parent e2687b0 commit 3535e2f

File tree

2 files changed

+175
-3
lines changed

2 files changed

+175
-3
lines changed

‎spec/LdapAuth.spec.js‎

Lines changed: 138 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,144 @@ 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('DN injection prevention', () => {
97+
it('should prevent DN injection via comma in authData.id', async done => {
98+
// Mock server accepts the DN that would result from an unescaped injection
99+
const server = await mockLdapServer(port, 'uid=admin,ou=admins,o=example');
100+
const options = {
101+
suffix: 'o=example',
102+
url: `ldap://localhost:${port}`,
103+
dn: 'uid={{id}}, o=example',
104+
};
105+
// Attacker tries to inject additional DN components via comma
106+
// Without escaping: DN = uid=admin,ou=admins, o=example (3 RDNs) → matches mock
107+
// With escaping: DN = uid=admin\,ou=admins, o=example (2 RDNs) → doesn't match
108+
try {
109+
await ldap.validateAuthData({ id: 'admin,ou=admins', password: 'secret' }, options);
110+
fail('Should have rejected DN injection attempt');
111+
} catch (err) {
112+
expect(err.message).toBe('LDAP: Wrong username or password');
113+
}
114+
server.close(done);
115+
});
116+
});
117+
118+
describe('Filter injection prevention', () => {
119+
it('should prevent LDAP filter injection via wildcard in authData.id', async done => {
120+
// Mock server accepts uid=*, o=example (the attacker's bind DN)
121+
// The * is not special in DNs so it binds fine regardless of escaping
122+
const server = await mockLdapServer(port, 'uid=*, o=example');
123+
const options = {
124+
suffix: 'o=example',
125+
url: `ldap://localhost:${port}`,
126+
dn: 'uid={{id}}, o=example',
127+
groupCn: 'powerusers',
128+
groupFilter: '(&(uniqueMember=uid={{id}}, o=example)(objectClass=groupOfUniqueNames))',
129+
};
130+
// Attacker uses * as ID to match any group member via wildcard
131+
// Group has member uid=testuser, not uid=*
132+
// Without escaping: filter uses SubstringFilter, matches testuser → passes
133+
// With escaping: filter uses EqualityFilter with literal \2a, no match → fails
134+
try {
135+
await ldap.validateAuthData({ id: '*', password: 'secret' }, options);
136+
fail('Should have rejected filter injection attempt');
137+
} catch (err) {
138+
expect(err.message).toBe('LDAP: User not in group');
139+
}
140+
server.close(done);
141+
});
142+
});
143+
});
144+
7145
describe('Ldap Auth', () => {
8146
it('Should fail with missing options', done => {
9147
ldap

‎src/Adapters/Auth/ldap.js‎

Lines changed: 37 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) => {
@@ -87,10 +118,11 @@ function validateAuthData(authData, options) {
87118
: { url: options.url };
88119

89120
const client = ldapjs.createClient(clientOptions);
121+
const escapedId = escapeDN(authData.id);
90122
const userCn =
91123
typeof options.dn === 'string'
92-
? options.dn.replace('{{id}}', authData.id)
93-
: `uid=${authData.id},${options.suffix}`;
124+
? options.dn.replace('{{id}}', escapedId)
125+
: `uid=${escapedId},${options.suffix}`;
94126

95127
return new Promise((resolve, reject) => {
96128
client.bind(userCn, authData.password, ldapError => {
@@ -140,7 +172,7 @@ function optionsAreValid(options) {
140172
}
141173

142174
function searchForGroup(client, options, id, resolve, reject) {
143-
const filter = options.groupFilter.replace(/{{id}}/gi, id);
175+
const filter = options.groupFilter.replace(/{{id}}/gi, escapeFilter(id));
144176
const opts = {
145177
scope: 'sub',
146178
filter: filter,
@@ -184,4 +216,6 @@ function validateAppId() {
184216
module.exports = {
185217
validateAppId,
186218
validateAuthData,
219+
escapeDN,
220+
escapeFilter,
187221
};

0 commit comments

Comments
 (0)
⚡