From 5b8cbb755b9662ae81aad8988c41f73f8c49e8bb Mon Sep 17 00:00:00 2001
From: Patrick Mooney <pmooney@pfmooney.com>
Date: Sat, 31 Oct 2015 09:01:50 -0500
Subject: [PATCH] Fix strict DN parsing in server

---
 lib/server.js       | 68 +++++++++++++++++++--------------------------
 test/server.test.js | 66 ++++++++++++++++++++++++++++++++++++++++++-
 2 files changed, 93 insertions(+), 41 deletions(-)

diff --git a/lib/server.js b/lib/server.js
index 74d8bc4..37439b9 100644
--- a/lib/server.js
+++ b/lib/server.js
@@ -124,45 +124,6 @@ function getResponse(req) {
 }
 
 
-function decodeDN(req, strict) {
-  assert.ok(req);
-  var parse;
-  if (strict) {
-    parse = dn.parse;
-  } else {
-    parse = function (input) {
-      try {
-        return dn.parse(input);
-      } catch (e) {
-        return input;
-      }
-    };
-  }
-  switch (req.protocolOp) {
-  case Protocol.LDAP_REQ_BIND:
-    req.name = parse(req.name);
-    break;
-  case Protocol.LDAP_REQ_ADD:
-  case Protocol.LDAP_REQ_COMPARE:
-  case Protocol.LDAP_REQ_DELETE:
-    req.entry = parse(req.entry);
-    break;
-  case Protocol.LDAP_REQ_MODIFY:
-    req.object = parse(req.object);
-    break;
-  case Protocol.LDAP_REQ_MODRDN:
-    req.entry = parse(req.entry);
-    // TODO: handle newRdn/Superior
-    break;
-  case Protocol.LDAP_REQ_SEARCH:
-    req.baseObject = parse(req.baseObject);
-    break;
-  default:
-    break;
-  }
-}
-
-
 function defaultHandler(req, res, next) {
   assert.ok(req);
   assert.ok(res);
@@ -396,7 +357,34 @@ function Server(options) {
       }
 
       // parse string DNs for routing/etc
-      decodeDN(req, this.strictDN);
+      try {
+        switch (req.protocolOp) {
+        case Protocol.LDAP_REQ_BIND:
+          req.name = dn.parse(req.name);
+          break;
+        case Protocol.LDAP_REQ_ADD:
+        case Protocol.LDAP_REQ_COMPARE:
+        case Protocol.LDAP_REQ_DELETE:
+          req.entry = dn.parse(req.entry);
+          break;
+        case Protocol.LDAP_REQ_MODIFY:
+          req.object = dn.parse(req.object);
+          break;
+        case Protocol.LDAP_REQ_MODRDN:
+          req.entry = dn.parse(req.entry);
+          // TODO: handle newRdn/Superior
+          break;
+        case Protocol.LDAP_REQ_SEARCH:
+          req.baseObject = dn.parse(req.baseObject);
+          break;
+        default:
+          break;
+        }
+      } catch (e) {
+        if (self.strictDN) {
+          return res.end(errors.LDAP_INVALID_DN_SYNTAX);
+        }
+      }
 
       res.connection = c;
       res.logId = req.logId;
diff --git a/test/server.test.js b/test/server.test.js
index d9125d8..5fec3e3 100644
--- a/test/server.test.js
+++ b/test/server.test.js
@@ -215,7 +215,71 @@ test('route unbind', function (t) {
   });
 });
 
-test('non-strict route', function (t) {
+test('strict routing', function (t) {
+  var testDN = 'cn=valid';
+  var clt;
+  vasync.pipeline({
+    funcs: [
+      function setup(_, cb) {
+        server = ldap.createServer({
+          // strictDN: true - on by default
+        });
+        sock = getSock();
+        // invalid DNs would go to default handler
+        server.search('', function (req, res, next) {
+          t.ok(req.dn);
+          t.equal(typeof (req.dn), 'object');
+          t.equal(req.dn.toString(), testDN);
+          res.end();
+          next();
+        });
+        server.listen(sock, function () {
+          t.ok(true, 'server startup');
+          clt = ldap.createClient({
+            socketPath: sock,
+            strictDN: false
+          });
+          cb();
+        });
+      },
+      function testBad(_, cb) {
+        clt.search('not a dn', {scope: 'base'}, function (err, res) {
+          t.ifError(err);
+          res.once('error', function (err2) {
+            t.ok(err2);
+            t.equal(err2.code, ldap.LDAP_INVALID_DN_SYNTAX);
+            cb();
+          });
+          res.once('end', function () {
+            t.fail('accepted invalid dn');
+            cb('bogus');
+          });
+        });
+      },
+      function testGood(_, cb) {
+        clt.search(testDN, {scope: 'base'}, function (err, res) {
+          t.ifError(err);
+          res.once('error', function (err2) {
+            t.ifError(err2);
+            cb(err2);
+          });
+          res.once('end', function (result) {
+            t.ok(result, 'accepted invalid dn');
+            cb();
+          });
+        });
+      }
+    ]
+  }, function (err, res) {
+    if (clt) {
+      clt.destroy();
+    }
+    server.close();
+    t.end();
+  });
+});
+
+test('non-strict routing', function (t) {
   server = ldap.createServer({
     strictDN: false
   });