ISSigTool: Finish.

Components/ISSigFunc.pas

@@ -78,8 +78,10 @@ uses
   StringScanner;
 
 const
-  ISSigTextFileLengthLimit = 500;
+  ISSigTextFileLengthLimit = 1500;
 
+  NonControlASCIICharsSet = [#32..#126];
+  UTF8HighCharsSet = [#128..#244];
   DigitsSet = ['0'..'9'];
   HexDigitsSet = DigitsSet + ['a'..'f'];
 
@@ -98,8 +100,11 @@ function CalcHashToSign(const AFileName: String; const AFileSize: Int64;
 begin
   var Context: TSHA256Context;
   SHA256Init(Context);
-  if AFileName <> '' then
-    SHA256Update(Context, Pointer(AFileName)^, Length(AFileName)*SizeOf(AFileName[1]));
+  const UTF8FileName = UTF8String(AFileName);
+  const UTF8FileNameLength: Cardinal = Length(UTF8FileName);
+  SHA256Update(Context, UTF8FileNameLength, SizeOf(UTF8FileNameLength));
+  if UTF8FileNameLength > 0 then
+    SHA256Update(Context, Pointer(UTF8FileName)^, UTF8FileNameLength*SizeOf(UTF8FileName[1]));
   SHA256Update(Context, AFileSize, SizeOf(AFileSize));
   SHA256Update(Context, AFileHash, SizeOf(AFileHash));
   Result := SHA256Final(Context);
@@ -112,15 +117,23 @@ end;
 
 function ConsumeLineValue(var SS: TStringScanner; const AIdent: String;
   var AValue: String; const AMinValueLength, AMaxValueLength: Integer;
-  const AAllowedChars: TSysCharSet = []): Boolean;
+  const AAllowedChars: TSysCharSet = []; const ARequireQuotes: Boolean = False): Boolean;
+var
+  DisallowedChars: TSysCharSet;
 begin
   Result := False;
-  if SS.Consume(AIdent) and SS.Consume(' ') then
-    if SS.ConsumeMultiToString(AAllowedChars, AValue, AMinValueLength,
+  if SS.Consume(AIdent) and SS.Consume(' ') and (not ARequireQuotes or SS.Consume('"')) then
+    if ARequireQuotes then
+      DisallowedChars := ['"']
+    else
+      DisallowedChars := [];
+    if SS.ConsumeMultiToString(AAllowedChars, DisallowedChars, AValue, AMinValueLength,
        AMaxValueLength) > 0 then begin
-      { CRLF and LF line breaks are allowed (but not CR) }
-      SS.Consume(#13);
-      Result := SS.Consume(#10);
+      if not ARequireQuotes or SS.Consume('"') then begin
+        { CRLF and LF line breaks are allowed (but not CR) }
+        SS.Consume(#13);
+        Result := SS.Consume(#10);
+      end;
     end;
 end;
 
@@ -147,8 +160,6 @@ begin
   { Defense-in-depth: Reject any non-CRLF control characters up front, as well
     as any non-ASCII and non-UTF8-high characters (to avoid any possible issues with
     converting invalid multibyte characters) }
-  const NonControlASCIICharsSet = [#32..#126];
-  const UTF8HighCharsSet = [#128..#244];
   for var C in U do
     if not (CharInSet(C, [#10, #13]) or CharInSet(C, NonControlASCIICharsSet) or
             CharInSet(C, UTF8HighCharsSet)) then
@@ -176,6 +187,16 @@ end;
 function ISSigCreateSignatureText(const AKey: TECDSAKey;
   const AFileName: String; const AFileSize: Int64; const AFileHash: TSHA256Digest): String;
 begin
+  { Ensure unverifiable signature files can't be created accidentally }
+  const UTF8FileName = UTF8String(AFileName);
+  if Length(UTF8FileName) > 1000 then
+    raise Exception.Create('File name is too long');
+  if String(UTF8FileName) <> AFileName then
+    raise Exception.Create('File name contains invalid surrogate pairs');
+  for var C in UTF8FileName do
+    if not (C in ((NonControlASCIICharsSet - ['"']) + UTF8HighCharsSet)) then { Note this rejects quotes }
+      raise Exception.Create('File name contains invalid characters');
+
   { File size is limited to 16 digits (enough for >9 EB) }
   if (AFileSize < 0) or (AFileSize > {$IF CompilerVersion >= 36.0} 9_999_999_999_999_999 {$ELSE} 9999999999999999 {$ENDIF} ) then
     raise Exception.Create('File size out of range');
@@ -189,7 +210,7 @@ begin
 
   Result := Format(
     'format issig-v2'#13#10 +
-    'file-name %s'#13#10 +
+    'file-name "%s"'#13#10 +
     'file-size %d'#13#10 +
     'file-hash %s'#13#10 +
     'key-id %s'#13#10 +
@@ -226,7 +247,7 @@ begin
   var SS := TStringScanner.Create(AText);
   if not ConsumeLineValue(SS, 'format', TextValues.Format, 8, 8) or
      ((TextValues.Format <> 'issig-v1') and ((TextValues.Format <> 'issig-v2'))) or
-     ((TextValues.Format = 'issig-v2') and not ConsumeLineValue(SS, 'file-name', TextValues.FileName, 1, MaxInt)) or
+     ((TextValues.Format = 'issig-v2') and not ConsumeLineValue(SS, 'file-name', TextValues.FileName, 1, MaxInt, [], True)) or
      not ConsumeLineValue(SS, 'file-size', TextValues.FileSize, 1, 16, DigitsSet) or
      not ConsumeLineValue(SS, 'file-hash', TextValues.FileHash, 64, 64, HexDigitsSet) or
      not ConsumeLineValue(SS, 'key-id', TextValues.KeyID, 64, 64, HexDigitsSet) or

Components/PathFunc.pas

@@ -19,6 +19,7 @@ function PathCharIsTrailByte(const S: String; const Index: Integer): Boolean;
 function PathCharLength(const S: String; const Index: Integer): Integer;
 function PathCombine(const Dir, Filename: String): String;
 function PathCompare(const S1, S2: String): Integer;
+function PathSame(const S1, S2: String): Boolean;
 function PathDrivePartLength(const Filename: String): Integer;
 function PathDrivePartLengthEx(const Filename: String;
   const IncludeSignificantSlash: Boolean): Integer;
@@ -157,6 +158,11 @@ begin
   Result := CompareStr(PathLowercase(S1), PathLowercase(S2));
 end;
 
+function PathSame(const S1, S2: String): Boolean;
+begin
+  Result := PathCompare(S1, S2) = 0;
+end;
+
 function PathDrivePartLength(const Filename: String): Integer;
 begin
   Result := PathDrivePartLengthEx(Filename, False);

Components/StringScanner.pas

@@ -25,9 +25,9 @@ type
     class function Create(const AString: String): TStringScanner; static;
     function Consume(const C: Char): Boolean; overload;
     function Consume(const S: String): Boolean; overload;
-    function ConsumeMulti(const C: TSysCharSet; const AMinChars: Integer = 1;
+    function ConsumeMulti(const AAllowedChars, ADisallowedChars: TSysCharSet; const AMinChars: Integer = 1;
       const AMaxChars: Integer = MaxInt; const AAllowControl: Boolean = False): Integer;
-    function ConsumeMultiToString(const C: TSysCharSet;
+    function ConsumeMultiToString(const AAllowedChars, ADisallowedChars: TSysCharSet;
       var ACapturedString: String; const AMinChars: Integer = 1;
       const AMaxChars: Integer = MaxInt; const AAllowControl: Boolean = False): Integer;
     property ReachedEnd: Boolean read GetReachedEnd;
@@ -71,8 +71,10 @@ begin
   Result := True;
 end;
 
-function TStringScanner.ConsumeMulti(const C: TSysCharSet;
+function TStringScanner.ConsumeMulti(const AAllowedChars, ADisallowedChars: TSysCharSet;
   const AMinChars, AMaxChars: Integer; const AAllowControl: Boolean): Integer;
+{ AAllowedChars may be empty to allow all chars and ADisallowedChars may be empty to
+  disallow none }
 begin
   if (AMinChars <= 0) or (AMinChars > AMaxChars) then
     raise Exception.Create('TStringScanner.ConsumeMulti: Invalid parameter');
@@ -83,7 +85,8 @@ begin
 
   Result := 0;
   while (Result < AMaxChars) and (Result < Remain) and
-     ((C = []) or CharInSet(FStr[FPosition + Result], C)) and
+     ((AAllowedChars = []) or CharInSet(FStr[FPosition + Result], AAllowedChars)) and
+     ((ADisallowedChars = []) or not CharInSet(FStr[FPosition + Result], ADisallowedChars)) and
      (AAllowControl or not FStr[FPosition + Result].IsControl) do
     Inc(Result);
 
@@ -93,11 +96,11 @@ begin
     Inc(FPosition, Result);
 end;
 
-function TStringScanner.ConsumeMultiToString(const C: TSysCharSet;
+function TStringScanner.ConsumeMultiToString(const AAllowedChars, ADisallowedChars: TSysCharSet;
   var ACapturedString: String; const AMinChars, AMaxChars: Integer; const AAllowControl: Boolean): Integer;
 begin
   const StartPos = FPosition;
-  Result := ConsumeMulti(C, AMinChars, AMaxChars, AAllowControl);
+  Result := ConsumeMulti(AAllowedChars, ADisallowedChars, AMinChars, AMaxChars, AAllowControl);
   if Result > 0 then
     ACapturedString := Copy(FStr, StartPos, Result)
   else

Projects/ISSigTool.dpr

@@ -193,7 +193,7 @@ begin
   const Verified = ISSigVerifySignature(AFilename, [AKey],
     ExistingFileName, ExistingFileSize, ExistingFileHash, nil, nil, nil);
 
-  if Verified and SameText(FileName, ExistingFileName) and (FileSize = ExistingFileSize) and
+  if Verified and PathSame(FileName, ExistingFileName) and (FileSize = ExistingFileSize) and
      SHA256DigestsEqual(FileHash, ExistingFileHash) then begin
     PrintUnlessQuiet('signature unchanged');
     Exit;
@@ -248,7 +248,7 @@ begin
   ) then
     Exit;
 
-  if (ExpectedFileName <> '') and not SameText(PathExtractName(AFilename), ExpectedFileName) then begin
+  if (ExpectedFileName <> '') and not PathSame(PathExtractName(AFilename), ExpectedFileName) then begin
     PrintUnlessQuiet('WRONGNAME (File name is incorrect)');
     Exit;
   end;