Test malformed DNs

2 files changed, 114 insertions, 13 deletions

diff --git a/lib/gitlab/ldap/dn.rb b/lib/gitlab/ldap/dn.rb
index 234de1fe7eb..048b669b13a 100644
--- a/lib/gitlab/ldap/dn.rb
+++ b/lib/gitlab/ldap/dn.rb
@@ -21,6 +21,9 @@
 # class also helps take care of that.
 module Gitlab
   module LDAP
+    MalformedDnError = Class.new(StandardError)
+    UnsupportedDnFormatError = Class.new(StandardError)
+
     class DN
       ##
       # Initialize a DN, escaping as required. Pass in attributes in name/value
@@ -69,19 +72,19 @@ module Gitlab
               state = :key_oid
               key << char
             when ' ' then state = :key
-            else raise "DN badly formed"
+            else raise(MalformedDnError, "Unrecognized first character of an RDN attribute type name \"#{char}\"")
             end
           when :key_normal then
             case char
             when '=' then state = :value
             when 'a'..'z', '0'..'9', '-', ' ' then key << char
-            else raise "DN badly formed"
+            else raise(MalformedDnError, "Unrecognized RDN attribute type name character \"#{char}\"")
             end
           when :key_oid then
             case char
             when '=' then state = :value
             when '0'..'9', '.', ' ' then key << char
-            else raise "DN badly formed"
+            else raise(MalformedDnError, "Unrecognized RDN OID attribute type name character \"#{char}\"")
             end
           when :value then
             case char
@@ -124,7 +127,7 @@ module Gitlab
             when '0'..'9', 'a'..'f' then
               state = :value_normal
               value << "#{hex_buffer}#{char}".to_i(16).chr
-            else raise "DN badly formed"
+            else raise(MalformedDnError, "Invalid escaped hex code \"\\#{hex_buffer}#{char}\"")
             end
           when :value_normal_escape_space then
             case char
@@ -157,7 +160,7 @@ module Gitlab
             when '0'..'9', 'a'..'f' then
               state = :value_quoted
               value << "#{hex_buffer}#{char}".to_i(16).chr
-            else raise "DN badly formed"
+            else raise(MalformedDnError, "Expected the second character of a hex pair inside a double quoted value, but got \"#{char}\"")
             end
           when :value_hexstring then
             case char
@@ -170,14 +173,14 @@ module Gitlab
               yield key.string.strip, value.string.rstrip
               key = StringIO.new
               value = StringIO.new;
-            else raise "DN badly formed"
+            else raise(MalformedDnError, "Expected the first character of a hex pair, but got \"#{char}\"")
             end
           when :value_hexstring_hex then
             case char
             when '0'..'9', 'a'..'f' then
               state = :value_hexstring
               value << char
-            else raise "DN badly formed"
+            else raise(MalformedDnError, "Expected the second character of a hex pair, but got \"#{char}\"")
             end
           when :value_end then
             case char
@@ -187,14 +190,14 @@ module Gitlab
               yield key.string.strip, value.string.rstrip
               key = StringIO.new
               value = StringIO.new;
-            else raise "DN badly formed"
+            else raise(MalformedDnError, "Expected the end of an attribute value, but got \"#{char}\"")
             end
           else raise "Fell out of state machine"
           end
         end
 
         # Last pair
-        raise "DN badly formed" unless
+        raise(MalformedDnError, 'DN string ended unexpectedly') unless
           [:value, :value_normal, :value_hexstring, :value_end].include? state
 
         yield key.string.strip, value.string.rstrip
diff --git a/spec/lib/gitlab/ldap/dn_spec.rb b/spec/lib/gitlab/ldap/dn_spec.rb
index 6b197fa22fd..d4fbe1c45ea 100644
--- a/spec/lib/gitlab/ldap/dn_spec.rb
+++ b/spec/lib/gitlab/ldap/dn_spec.rb
@@ -71,16 +71,114 @@ describe Gitlab::LDAP::DN do
     end
 
     context 'when the given DN is malformed' do
-      let(:given) { 'uid\\=john' }
+      context 'when ending with a comma' do
+        let(:given) { 'uid=John Smith,' }
 
-      it 'raises MalformedDnError' do
-        expect(subject).to raise_error(MalformedDnError)
+        it 'raises MalformedDnError' do
+          expect{ subject }.to raise_error(Gitlab::LDAP::MalformedDnError, 'DN string ended unexpectedly')
+        end
+      end
+
+      context 'when given a BER encoded attribute value with a space in it' do
+        let(:given) { '0.9.2342.19200300.100.1.25=#aa aa' }
+
+        it 'raises MalformedDnError' do
+          expect{ subject }.to raise_error(Gitlab::LDAP::MalformedDnError, "Expected the end of an attribute value, but got \"a\"")
+        end
+      end
+
+      context 'when given a BER encoded attribute value with a non-hex character in it' do
+        let(:given) { '0.9.2342.19200300.100.1.25=#aaXaaa' }
+
+        it 'raises MalformedDnError' do
+          expect{ subject }.to raise_error(Gitlab::LDAP::MalformedDnError, "Expected the first character of a hex pair, but got \"x\"")
+        end
+      end
+
+      context 'when given a BER encoded attribute value with a non-hex character in it' do
+        let(:given) { '0.9.2342.19200300.100.1.25=#aaaYaa' }
+
+        it 'raises MalformedDnError' do
+          expect{ subject }.to raise_error(Gitlab::LDAP::MalformedDnError, "Expected the second character of a hex pair, but got \"y\"")
+        end
+      end
+
+      context 'when given a hex pair with a non-hex character in it, inside double quotes' do
+        let(:given) { 'uid="Sebasti\\cX\\a1n"' }
+
+        it 'raises MalformedDnError' do
+          expect{ subject }.to raise_error(Gitlab::LDAP::MalformedDnError, "Expected the second character of a hex pair inside a double quoted value, but got \"x\"")
+        end
+      end
+
+      context 'without a name value pair' do
+        let(:given) { 'John' }
+
+        it 'raises MalformedDnError' do
+          expect{ subject }.to raise_error(Gitlab::LDAP::MalformedDnError, 'DN string ended unexpectedly')
+        end
+      end
+
+      context 'with an open (as opposed to closed) double quote' do
+        let(:given) { 'cn="James' }
+
+        it 'raises MalformedDnError' do
+          expect{ subject }.to raise_error(Gitlab::LDAP::MalformedDnError, 'DN string ended unexpectedly')
+        end
+      end
+
+      context 'with an invalid escaped hex code' do
+        let(:given) { 'cn=J\ames' }
+
+        it 'raises MalformedDnError' do
+          expect{ subject }.to raise_error(Gitlab::LDAP::MalformedDnError, 'Invalid escaped hex code "\am"')
+        end
+      end
+
+      context 'with a value ending with the escape character' do
+        let(:given) { 'cn=\\' }
+
+        it 'raises MalformedDnError' do
+          expect{ subject }.to raise_error(Gitlab::LDAP::MalformedDnError, 'DN string ended unexpectedly')
+        end
+      end
+
+      context 'with an invalid OID attribute type name' do
+        let(:given) { '1.2.d=Value' }
+
+        it 'raises MalformedDnError' do
+          expect{ subject }.to raise_error(Gitlab::LDAP::MalformedDnError, 'Unrecognized RDN OID attribute type name character "d"')
+        end
+      end
+
+      context 'with a period in a non-OID attribute type name' do
+        let(:given) { 'd1.2=Value' }
+
+        it 'raises MalformedDnError' do
+          expect{ subject }.to raise_error(Gitlab::LDAP::MalformedDnError, 'Unrecognized RDN attribute type name character "."')
+        end
+      end
+
+      context 'when starting with non-space, non-alphanumeric character' do
+        let(:given) { ' -uid=John Smith' }
+
+        it 'raises MalformedDnError' do
+          expect{ subject }.to raise_error(Gitlab::LDAP::MalformedDnError, 'Unrecognized first character of an RDN attribute type name "-"')
+        end
+      end
+
+      context 'when given a UID with an escaped equal sign' do
+        let(:given) { 'uid\\=john' }
+
+        it 'raises MalformedDnError' do
+          expect{ subject }.to raise_error(Gitlab::LDAP::MalformedDnError, 'Unrecognized RDN attribute type name character "\\"')
+        end
       end
     end
   end
 
   def assert_generic_test(test_description, got, expected)
-    test_failure_message = "Failed test description: '#{test_description}'\n\n    expected: #{expected}\n         got: #{got}"
+    test_failure_message = "Failed test description: '#{test_description}'\n\n    expected: \"#{expected}\"\n         got: \"#{got}\""
     expect(got).to eq(expected), test_failure_message
   end
 end