Browse Source

Enforce no leading zeroes in OID branches (longer than 1 character)

Ugochukwu Mmaduekwe 6 years ago
parent
commit
7ca1e9abeb
2 changed files with 56 additions and 75 deletions
  1. 35 47
      CryptoLib.Tests/src/Asn1/OIDTests.pas
  2. 21 28
      CryptoLib/src/Asn1/ClpAsn1Objects.pas

+ 35 - 47
CryptoLib.Tests/src/Asn1/OIDTests.pas

@@ -43,15 +43,13 @@ type
   private
 
   var
-    Foid: string;
     Freq1, Freq2: TBytes;
 
     procedure DoRecodeCheck(const oid: String; const enc: TBytes);
     procedure DoValidOidCheck(const oid: String);
-    procedure DoInvalidOidCheck;
+    procedure DoInvalidOidCheck(const oid: String);
     procedure DoBranchCheck(const stem, branch: String);
     procedure DoOnCheck(const stem, test: String; expected: Boolean);
-    procedure DoConstructorMethod;
 
   protected
     procedure SetUp; override;
@@ -78,23 +76,18 @@ begin
     + branch);
 end;
 
-procedure TTestOID.DoConstructorMethod;
+procedure TTestOID.DoInvalidOidCheck(const oid: String);
 begin
-  TDerObjectIdentifier.Create(Foid);
-end;
-
-procedure TTestOID.DoInvalidOidCheck;
-var
-{$IFNDEF FPC}
-  Method: TTestMethod;
-{$ELSE}
-  Method: TRunMethod;
-{$ENDIF FPC}
-begin
-
-  Method := DoConstructorMethod;
-  CheckException(Method, EFormatCryptoLibException,
-    'Expected "EFormatCryptoLibException" But None Gotten');
+  try
+    TDerObjectIdentifier.Create(oid);
+    Fail(Format('failed to catch bad oid: %s', [oid]));
+
+  except
+    on e: EFormatCryptoLibException do
+    begin
+      // expected
+    end;
+  end;
 end;
 
 procedure TTestOID.DoOnCheck(const stem, test: String; expected: Boolean);
@@ -145,40 +138,35 @@ begin
   DoRecodeCheck('1.2.54.34359733987.17', Freq2);
 
   DoValidOidCheck('0.1');
+  DoValidOidCheck('1.0');
+  DoValidOidCheck('1.0.2');
+  DoValidOidCheck('1.0.20');
+  DoValidOidCheck('1.0.200');
   DoValidOidCheck
     ('1.1.127.32512.8323072.2130706432.545460846592.139637976727552.35747322042253312.9151314442816847872');
   DoValidOidCheck('1.2.123.12345678901.1.1.1');
 
   DoValidOidCheck('2.25.196556539987194312349856245628873852187.1');
 
-  Foid := '0';
-  DoInvalidOidCheck;
-  Foid := '1';
-  DoInvalidOidCheck;
-  Foid := '2';
-  DoInvalidOidCheck;
-  Foid := '3.1';
-  DoInvalidOidCheck;
-  Foid := '..1';
-  DoInvalidOidCheck;
-  Foid := '192.168.1.1';
-  DoInvalidOidCheck;
-  Foid := '.123452';
-  DoInvalidOidCheck;
-  Foid := '1.';
-  DoInvalidOidCheck;
-  Foid := '1.345.23.34..234';
-  DoInvalidOidCheck;
-  Foid := '1.345.23.34.234.';
-  DoInvalidOidCheck;
-  Foid := '.12.345.77.234';
-  DoInvalidOidCheck;
-  Foid := '.12.345.77.234.';
-  DoInvalidOidCheck;
-  Foid := '1.2.3.4.A.5';
-  DoInvalidOidCheck;
-  Foid := '1,2';
-  DoInvalidOidCheck;
+  DoInvalidOidCheck('0');
+  DoInvalidOidCheck('1');
+  DoInvalidOidCheck('2');
+  DoInvalidOidCheck('3.1');
+  DoInvalidOidCheck('0.01');
+  DoInvalidOidCheck('00.1');
+  DoInvalidOidCheck('1.00.2');
+  DoInvalidOidCheck('1.0.02');
+  DoInvalidOidCheck('1.2.00');
+  DoInvalidOidCheck('..1');
+  DoInvalidOidCheck('192.168.1.1');
+  DoInvalidOidCheck('.123452');
+  DoInvalidOidCheck('1.');
+  DoInvalidOidCheck('1.345.23.34..234');
+  DoInvalidOidCheck('1.345.23.34.234.');
+  DoInvalidOidCheck('.12.345.77.234');
+  DoInvalidOidCheck('.12.345.77.234.');
+  DoInvalidOidCheck('1.2.3.4.A.5');
+  DoInvalidOidCheck('1,2');
 
   DoBranchCheck('1.1', '2.2');
 

+ 21 - 28
CryptoLib/src/Asn1/ClpAsn1Objects.pas

@@ -4838,51 +4838,48 @@ end;
 class function TDerObjectIdentifier.IsValidBranchID(const branchID: String;
   start: Int32): Boolean;
 var
-  periodAllowed: Boolean;
-  Pos: Int32;
+  digitCount, Pos: Int32;
   ch: Char;
 begin
-  periodAllowed := False;
+  digitCount := 0;
 
   Pos := System.length(branchID) + 1;
   System.Dec(Pos);
+
   while (Pos >= start) do
   begin
     ch := branchID[Pos];
 
-    // TODO Leading zeroes?
-    // if (('0' <= ch) and (ch <= '9')) then
-    // begin
-    // periodAllowed := true;
-    // continue;
-    // end;
-
-    // TODO Leading zeroes?
-    if (CharInSet(ch, ['0' .. '9'])) then
-    begin
-      periodAllowed := True;
-      System.Dec(Pos);
-      continue;
-    end;
-
     if (ch = '.') then
     begin
-      if (not(periodAllowed)) then
+      if ((digitCount = 0) or ((digitCount > 1) and (branchID[Pos + 1] = '0')))
+      then
       begin
         result := False;
         Exit;
       end;
 
-      periodAllowed := False;
-      System.Dec(Pos);
-      continue;
+      digitCount := 0;
+    end
+    else if (CharInSet(ch, ['0' .. '9'])) then
+    begin
+      System.Inc(digitCount);
+    end
+    else
+    begin
+      result := False;
+      Exit;
     end;
+    System.Dec(Pos);
+  end;
 
+  if ((digitCount = 0) or ((digitCount > 1) and (branchID[Pos + 1] = '0'))) then
+  begin
     result := False;
     Exit;
   end;
 
-  result := periodAllowed;
+  result := True;
 end;
 
 class function TDerObjectIdentifier.IsValidIdentifier(const identifier
@@ -4897,11 +4894,7 @@ begin
   end;
 
   first := identifier[1];
-  // if ((first < '0') or (first > '2')) then
-  // begin
-  // result := false;
-  // Exit;
-  // end;
+
   if (not CharInSet(first, ['0' .. '2'])) then
   begin
     result := False;