From 1dd0343860eb7e206fc5a01d75ff4d02eec364e9 Mon Sep 17 00:00:00 2001 From: Clement Michaud Date: Sat, 14 Oct 2017 13:49:25 +0200 Subject: [PATCH] Add input sanitizer to LDAP client to protect against LDAP injections --- server/src/lib/Server.ts | 1 - server/src/lib/ldap/ClientFactory.ts | 5 +- server/src/lib/ldap/InputsSanitizer.ts | 25 ++++++++ server/src/lib/ldap/SanitizedClient.ts | 63 ++++++++++++++++++++ server/test/ldap/InputsSanitizer.test.ts | 25 ++++++++ server/test/ldap/SanitizedClient.test.ts | 76 ++++++++++++++++++++++++ 6 files changed, 192 insertions(+), 3 deletions(-) create mode 100644 server/src/lib/ldap/InputsSanitizer.ts create mode 100644 server/src/lib/ldap/SanitizedClient.ts create mode 100644 server/test/ldap/InputsSanitizer.test.ts create mode 100644 server/test/ldap/SanitizedClient.test.ts diff --git a/server/src/lib/Server.ts b/server/src/lib/Server.ts index 436571fc..5d80e332 100644 --- a/server/src/lib/Server.ts +++ b/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"; diff --git a/server/src/lib/ldap/ClientFactory.ts b/server/src/lib/ldap/ClientFactory.ts index 48c4aeba..2f2b9217 100644 --- a/server/src/lib/ldap/ClientFactory.ts +++ b/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, - this.dovehash, this.logger); + return new SanitizedClient(new Client(userDN, password, this.config, this.ldapClientFactory, + this.dovehash, this.logger)); } } \ No newline at end of file diff --git a/server/src/lib/ldap/InputsSanitizer.ts b/server/src/lib/ldap/InputsSanitizer.ts new file mode 100644 index 00000000..ab6152de --- /dev/null +++ b/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; + } +} \ No newline at end of file diff --git a/server/src/lib/ldap/SanitizedClient.ts b/server/src/lib/ldap/SanitizedClient.ts new file mode 100644 index 00000000..543557a6 --- /dev/null +++ b/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 { + return this.client.open(); + } + + close(): BluebirdPromise { + return this.client.close(); + } + + searchGroups(username: string): BluebirdPromise { + 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 { + 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 { + 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 { + try { + const sanitizedUsername = InputsSanitizer.sanitize(username); + return this.client.modifyPassword(sanitizedUsername, newPassword); + } + catch (e) { + return BluebirdPromise.reject(new Error(SPECIAL_CHAR_USED_MESSAGE)); + } + } +} diff --git a/server/test/ldap/InputsSanitizer.test.ts b/server/test/ldap/InputsSanitizer.test.ts new file mode 100644 index 00000000..d991d0cd --- /dev/null +++ b/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 { 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); + }); +}); diff --git a/server/test/ldap/SanitizedClient.test.ts b/server/test/ldap/SanitizedClient.test.ts new file mode 100644 index 00000000..08aa8c80 --- /dev/null +++ b/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"); + }); + }); +}); \ No newline at end of file