fix: LDAP injection via unsanitized user input in DN and group filter construction (GHSA-7m6r-fhh7-r47c)

spec/LdapAuth.spec.js

@@ -4,6 +4,144 @@ const fs = require('fs');
 const port = 12345;
 const sslport = 12346;
 
+describe('LDAP Injection Prevention', () => {
+  describe('escapeDN', () => {
+    it('should escape comma', () => {
+      expect(ldap.escapeDN('admin,ou=evil')).toBe('admin\\,ou\\=evil');
+    });
+
+    it('should escape equals sign', () => {
+      expect(ldap.escapeDN('admin=evil')).toBe('admin\\=evil');
+    });
+
+    it('should escape plus sign', () => {
+      expect(ldap.escapeDN('admin+evil')).toBe('admin\\+evil');
+    });
+
+    it('should escape less-than and greater-than signs', () => {
+      expect(ldap.escapeDN('admin<evil>')).toBe('admin\\<evil\\>');
+    });
+
+    it('should escape hash at start', () => {
+      expect(ldap.escapeDN('#admin')).toBe('\\#admin');
+    });
+
+    it('should escape semicolon', () => {
+      expect(ldap.escapeDN('admin;evil')).toBe('admin\\;evil');
+    });
+
+    it('should escape double quote', () => {
+      expect(ldap.escapeDN('admin"evil')).toBe('admin\\"evil');
+    });
+
+    it('should escape backslash', () => {
+      expect(ldap.escapeDN('admin\\evil')).toBe('admin\\\\evil');
+    });
+
+    it('should escape leading space', () => {
+      expect(ldap.escapeDN(' admin')).toBe('\\ admin');
+    });
+
+    it('should escape trailing space', () => {
+      expect(ldap.escapeDN('admin ')).toBe('admin\\ ');
+    });
+
+    it('should escape multiple special characters', () => {
+      expect(ldap.escapeDN('admin,ou=evil+cn=x')).toBe('admin\\,ou\\=evil\\+cn\\=x');
+    });
+
+    it('should not modify safe values', () => {
+      expect(ldap.escapeDN('testuser')).toBe('testuser');
+      expect(ldap.escapeDN('john.doe')).toBe('john.doe');
+      expect(ldap.escapeDN('user123')).toBe('user123');
+    });
+  });
+
+  describe('escapeFilter', () => {
+    it('should escape asterisk', () => {
+      expect(ldap.escapeFilter('*')).toBe('\\2a');
+    });
+
+    it('should escape open parenthesis', () => {
+      expect(ldap.escapeFilter('test(')).toBe('test\\28');
+    });
+
+    it('should escape close parenthesis', () => {
+      expect(ldap.escapeFilter('test)')).toBe('test\\29');
+    });
+
+    it('should escape backslash', () => {
+      expect(ldap.escapeFilter('test\\')).toBe('test\\5c');
+    });
+
+    it('should escape null byte', () => {
+      expect(ldap.escapeFilter('test\x00')).toBe('test\\00');
+    });
+
+    it('should escape multiple special characters', () => {
+      expect(ldap.escapeFilter('*()\\')).toBe('\\2a\\28\\29\\5c');
+    });
+
+    it('should not modify safe values', () => {
+      expect(ldap.escapeFilter('testuser')).toBe('testuser');
+      expect(ldap.escapeFilter('john.doe')).toBe('john.doe');
+      expect(ldap.escapeFilter('user123')).toBe('user123');
+    });
+
+    it('should escape filter injection attempt with wildcard', () => {
+      expect(ldap.escapeFilter('x)(|(objectClass=*)')).toBe('x\\29\\28|\\28objectClass=\\2a\\29');
+    });
+  });
+
+  describe('DN injection prevention', () => {
+    it('should prevent DN injection via comma in authData.id', async done => {
+      // Mock server accepts the DN that would result from an unescaped injection
+      const server = await mockLdapServer(port, 'uid=admin,ou=admins,o=example');
+      const options = {
+        suffix: 'o=example',
+        url: `ldap://localhost:${port}`,
+        dn: 'uid={{id}}, o=example',
+      };
+      // Attacker tries to inject additional DN components via comma
+      // Without escaping: DN = uid=admin,ou=admins, o=example (3 RDNs) → matches mock
+      // With escaping: DN = uid=admin\,ou=admins, o=example (2 RDNs) → doesn't match
+      try {
+        await ldap.validateAuthData({ id: 'admin,ou=admins', password: 'secret' }, options);
+        fail('Should have rejected DN injection attempt');
+      } catch (err) {
+        expect(err.message).toBe('LDAP: Wrong username or password');
+      }
+      server.close(done);
+    });
+  });
+
+  describe('Filter injection prevention', () => {
+    it('should prevent LDAP filter injection via wildcard in authData.id', async done => {
+      // Mock server accepts uid=*, o=example (the attacker's bind DN)
+      // The * is not special in DNs so it binds fine regardless of escaping
+      const server = await mockLdapServer(port, 'uid=*, o=example');
+      const options = {
+        suffix: 'o=example',
+        url: `ldap://localhost:${port}`,
+        dn: 'uid={{id}}, o=example',
+        groupCn: 'powerusers',
+        groupFilter: '(&(uniqueMember=uid={{id}}, o=example)(objectClass=groupOfUniqueNames))',
+      };
+      // Attacker uses * as ID to match any group member via wildcard
+      // Group has member uid=testuser, not uid=*
+      // Without escaping: filter uses SubstringFilter, matches testuser → passes
+      // With escaping: filter uses EqualityFilter with literal \2a, no match → fails
+      try {
+        await ldap.validateAuthData({ id: '*', password: 'secret' }, options);
+        fail('Should have rejected filter injection attempt');
+      } catch (err) {
+        expect(err.message).toBe('LDAP: User not in group');
+      }
+      server.close(done);
+    });
+  });
+});
+
 describe('Ldap Auth', () => {
   it('Should fail with missing options', done => {
     ldap

src/Adapters/Auth/ldap.js

@@ -76,6 +76,37 @@
 const ldapjs = require('ldapjs');
 const Parse = require('parse/node').Parse;
 
+// Escape LDAP DN special characters per RFC 4514
+// https://datatracker.ietf.org/doc/html/rfc4514#section-2.4
+function escapeDN(value) {
+  let escaped = value
+    .replace(/\\/g, '\\\\')
+    .replace(/,/g, '\\,')
+    .replace(/=/g, '\\=')
+    .replace(/\+/g, '\\+')
+    .replace(/</g, '\\<')
+    .replace(/>/g, '\\>')
+    .replace(/#/g, '\\#')
+    .replace(/;/g, '\\;')
+    .replace(/"/g, '\\"');
+  if (escaped.startsWith(' ')) {
+    escaped = '\\ ' + escaped.slice(1);
+  }
+  if (escaped.endsWith(' ')) {
+    escaped = escaped.slice(0, -1) + '\\ ';
+  }
+  return escaped;
+}
+
+// Escape LDAP filter special characters per RFC 4515
+// https://datatracker.ietf.org/doc/html/rfc4515#section-3
+function escapeFilter(value) {
+  // eslint-disable-next-line no-control-regex
+  return value.replace(/[\\*()\x00]/g, ch =>
+    '\\' + ch.charCodeAt(0).toString(16).padStart(2, '0')
+  );
+}
+
 function validateAuthData(authData, options) {
   if (!optionsAreValid(options)) {
     return new Promise((_, reject) => {
@@ -87,10 +118,11 @@ function validateAuthData(authData, options) {
     : { url: options.url };
 
   const client = ldapjs.createClient(clientOptions);
+  const escapedId = escapeDN(authData.id);
   const userCn =
     typeof options.dn === 'string'
-      ? options.dn.replace('{{id}}', authData.id)
-      : `uid=${authData.id},${options.suffix}`;
+      ? options.dn.replace('{{id}}', escapedId)
+      : `uid=${escapedId},${options.suffix}`;
 
   return new Promise((resolve, reject) => {
     client.bind(userCn, authData.password, ldapError => {
@@ -140,7 +172,7 @@ function optionsAreValid(options) {
 }
 
 function searchForGroup(client, options, id, resolve, reject) {
-  const filter = options.groupFilter.replace(/{{id}}/gi, id);
+  const filter = options.groupFilter.replace(/{{id}}/gi, escapeFilter(id));
   const opts = {
     scope: 'sub',
     filter: filter,
@@ -184,4 +216,6 @@ function validateAppId() {
 module.exports = {
   validateAppId,
   validateAuthData,
+  escapeDN,
+  escapeFilter,
 };