Merge pull request #143 from clems4ever/protect-ldap-injection

Add input sanitizer to LDAP client to protect against LDAP injections

server/src/lib/Server.ts

@@ -10,7 +10,6 @@ import { ConfigurationParser } from "./configuration/ConfigurationParser";
 import { TOTPValidator } from "./TOTPValidator";
 import { TOTPGenerator } from "./TOTPGenerator";
 import { RestApi } from "./RestApi";
-import { Client } from "./ldap/Client";
 import { ServerVariablesHandler } from "./ServerVariablesHandler";
 import { SessionConfigurationBuilder } from "./configuration/SessionConfigurationBuilder";
 import { GlobalLogger } from "./logging/GlobalLogger";

server/src/lib/ldap/Client.ts

@@ -10,6 +10,7 @@ import { ILdapClient } from "./ILdapClient";
 import { ILdapClientFactory } from "./ILdapClientFactory";
 import { LdapConfiguration } from "../configuration/Configuration";
 import { Winston } from "../../../types/Dependencies";
+import Util = require("util");
 
 
 export class Client implements IClient {
@@ -75,8 +76,12 @@ export class Client implements IClient {
     that.logger.debug("LDAP: searching for user dn of %s", username);
     return that.ldapClient.searchAsync(this.options.users_dn, query)
       .then(function (users: { dn: string }[]) {
+        if (users.length > 0) {
           that.logger.debug("LDAP: retrieved user dn is %s", users[0].dn);
           return BluebirdPromise.resolve(users[0].dn);
+        }
+        return BluebirdPromise.reject(new Error(
+          Util.format("No user DN found for user '%s'", username)));
       });
   }
 

server/src/lib/ldap/ClientFactory.ts

@@ -1,6 +1,7 @@
 import { IClientFactory } from "./IClientFactory";
 import { IClient } from "./IClient";
 import { Client } from "./Client";
+import { SanitizedClient } from "./SanitizedClient";
 import { ILdapClientFactory } from "./ILdapClientFactory";
 import { LdapConfiguration } from "../configuration/Configuration";
 
@@ -23,7 +24,7 @@ export class ClientFactory implements IClientFactory {
   }
 
   create(userDN: string, password: string): IClient {
-    return new Client(userDN, password, this.config, this.ldapClientFactory,
+    return new SanitizedClient(new Client(userDN, password, this.config, this.ldapClientFactory,
-      this.dovehash, this.logger);
+      this.dovehash, this.logger));
   }
 }

server/src/lib/ldap/InputsSanitizer.ts

@@ -0,0 +1,25 @@
+
+// returns true for 1 or more matches, where 'a' is an array and 'b' is a search string or an array of multiple search strings
+function contains(a: string, character: string) {
+  // string match
+  return a.indexOf(character) > -1;
+}
+
+function containsOneOf(s: string, characters: string[]) {
+  return characters
+    .map((character: string) => { return contains(s, character); })
+    .reduce((acc: boolean, current: boolean) => { return acc || current; }, false);
+}
+
+export class InputsSanitizer {
+  static sanitize(input: string): string {
+    const forbiddenChars = [",", "\\", "'", "#", "+", "<", ">", ";", "\"", "="];
+    if (containsOneOf(input, forbiddenChars))
+      throw new Error("Input containing unsafe characters.");
+
+    if (input != input.trim())
+      throw new Error("Input has unexpected spaces.");
+
+    return input;
+  }
+}

server/src/lib/ldap/SanitizedClient.ts

@@ -0,0 +1,63 @@
+import BluebirdPromise = require("bluebird");
+import { IClient, GroupsAndEmails } from "./IClient";
+import { Client } from "./Client";
+import { InputsSanitizer } from "./InputsSanitizer";
+
+const SPECIAL_CHAR_USED_MESSAGE = "Special character used in LDAP query.";
+
+
+export class SanitizedClient implements IClient {
+  private client: IClient;
+
+  constructor(client: IClient) {
+    this.client = client;
+  }
+
+  open(): BluebirdPromise<void> {
+    return this.client.open();
+  }
+
+  close(): BluebirdPromise<void> {
+    return this.client.close();
+  }
+
+  searchGroups(username: string): BluebirdPromise<string[]> {
+    try {
+      const sanitizedUsername = InputsSanitizer.sanitize(username);
+      return this.client.searchGroups(sanitizedUsername);
+    }
+    catch (e) {
+      return BluebirdPromise.reject(new Error(SPECIAL_CHAR_USED_MESSAGE));
+    }
+  }
+
+  searchUserDn(username: string): BluebirdPromise<string> {
+    try {
+      const sanitizedUsername = InputsSanitizer.sanitize(username);
+      return this.client.searchUserDn(sanitizedUsername);
+    }
+    catch (e) {
+      return BluebirdPromise.reject(new Error(SPECIAL_CHAR_USED_MESSAGE));
+    }
+  }
+
+  searchEmails(username: string): BluebirdPromise<string[]> {
+    try {
+      const sanitizedUsername = InputsSanitizer.sanitize(username);
+      return this.client.searchEmails(sanitizedUsername);
+    }
+    catch (e) {
+      return BluebirdPromise.reject(new Error(SPECIAL_CHAR_USED_MESSAGE));
+    }
+  }
+
+  modifyPassword(username: string, newPassword: string): BluebirdPromise<void> {
+    try {
+      const sanitizedUsername = InputsSanitizer.sanitize(username);
+      return this.client.modifyPassword(sanitizedUsername, newPassword);
+    }
+    catch (e) {
+      return BluebirdPromise.reject(new Error(SPECIAL_CHAR_USED_MESSAGE));
+    }
+  }
+}

server/test/ldap/InputsSanitizer.test.ts

@@ -0,0 +1,25 @@
+import Assert = require("assert");
+import { InputsSanitizer } from "../../src/lib/ldap/InputsSanitizer";
+
+describe("test InputsSanitizer", function () {
+  it("should fail when special characters are used", function () {
+    Assert.throws(() => { InputsSanitizer.sanitize("ab,c"); }, Error);
+    Assert.throws(() => { InputsSanitizer.sanitize("a\\bc"); }, Error);
+    Assert.throws(() => { InputsSanitizer.sanitize("a'bc"); }, Error);
+    Assert.throws(() => { InputsSanitizer.sanitize("a#bc"); }, Error);
+    Assert.throws(() => { InputsSanitizer.sanitize("a+bc"); }, Error);
+    Assert.throws(() => { InputsSanitizer.sanitize("a<bc"); }, Error);
+    Assert.throws(() => { InputsSanitizer.sanitize("a>bc"); }, Error);
+    Assert.throws(() => { InputsSanitizer.sanitize("a;bc"); }, Error);
+    Assert.throws(() => { InputsSanitizer.sanitize("a\"bc"); }, Error);
+    Assert.throws(() => { InputsSanitizer.sanitize("a=bc"); }, Error);
+  });
+
+  it("should return original string", function () {
+    Assert.equal(InputsSanitizer.sanitize("abcdef"), "abcdef");
+  });
+
+  it("should trim", function () {
+    Assert.throws(() => { InputsSanitizer.sanitize("    abc    "); }, Error);
+  });
+});

server/test/ldap/SanitizedClient.test.ts

@@ -0,0 +1,76 @@
+import BluebirdPromise = require("bluebird");
+import { ClientStub } from "../mocks/ldap/ClientStub";
+import { SanitizedClient } from "../../src/lib/ldap/SanitizedClient";
+
+describe("test SanitizedClient", function () {
+  let client: SanitizedClient;
+
+  beforeEach(function () {
+    const clientStub = new ClientStub();
+    clientStub.searchUserDnStub.onCall(0).returns(BluebirdPromise.resolve());
+    clientStub.searchGroupsStub.onCall(0).returns(BluebirdPromise.resolve());
+    clientStub.searchEmailsStub.onCall(0).returns(BluebirdPromise.resolve());
+    clientStub.modifyPasswordStub.onCall(0).returns(BluebirdPromise.resolve());
+    client = new SanitizedClient(clientStub);
+  });
+
+  describe("special chars are used", function () {
+    it("should fail when special chars are used in searchUserDn", function () {
+      // potential ldap injection";
+      return client.searchUserDn("cn=dummy_user,ou=groupgs")
+        .then(function () {
+          return BluebirdPromise.reject(new Error("Should not be here."));
+        }, function () {
+          return BluebirdPromise.resolve();
+        });
+    });
+
+    it("should fail when special chars are used in searchGroups", function () {
+      // potential ldap injection";
+      return client.searchGroups("cn=dummy_user,ou=groupgs")
+        .then(function () {
+          return BluebirdPromise.reject(new Error("Should not be here."));
+        }, function () {
+          return BluebirdPromise.resolve();
+        });
+    });
+
+    it("should fail when special chars are used in searchEmails", function () {
+      // potential ldap injection";
+      return client.searchEmails("cn=dummy_user,ou=groupgs")
+        .then(function () {
+          return BluebirdPromise.reject(new Error("Should not be here."));
+        }, function () {
+          return BluebirdPromise.resolve();
+        });
+    });
+
+    it("should fail when special chars are used in modifyPassword", function () {
+      // potential ldap injection";
+      return client.modifyPassword("cn=dummy_user,ou=groupgs", "abc")
+        .then(function () {
+          return BluebirdPromise.reject(new Error("Should not be here."));
+        }, function () {
+          return BluebirdPromise.resolve();
+        });
+    });
+  });
+
+  describe("no special chars are used", function() {
+    it("should succeed when no special chars are used in searchUserDn", function () {
+      return client.searchUserDn("dummy_user");
+    });
+
+    it("should succeed when no special chars are used in searchGroups", function () {
+      return client.searchGroups("dummy_user");
+    });
+
+    it("should succeed when no special chars are used in searchEmails", function () {
+      return client.searchEmails("dummy_user");
+    });
+
+    it("should succeed when no special chars are used in modifyPassword", function () {
+      return client.modifyPassword("dummy_user", "abc");
+    });
+  });
+});