Browse Source

Add ValidateAndCombinePath check. When the DestDir isn't known yet (one place: getting size of files) then it only validates the filename itself and doesnt combine.

Martijn Laan 3 months ago
parent
commit
d48febb698

+ 26 - 15
Components/PathFunc.pas

@@ -47,7 +47,8 @@ function PathStrScan(const S: PChar; const C: Char): PChar;
 function RemoveBackslash(const S: String): String;
 function RemoveBackslash(const S: String): String;
 function RemoveBackslashUnlessRoot(const S: String): String;
 function RemoveBackslashUnlessRoot(const S: String): String;
 function ValidateAndCombinePath(const ADestDir, AFilename: String;
 function ValidateAndCombinePath(const ADestDir, AFilename: String;
-  out AResultingPath: String): Boolean;
+  out AResultingPath: String): Boolean; overload;
+function ValidateAndCombinePath(const ADestDir, AFilename: String): Boolean; overload;
 
 
 implementation
 implementation
 
 
@@ -611,7 +612,8 @@ function ValidateAndCombinePath(const ADestDir, AFilename: String;
   Returns True if all security checks pass, with the combination of ADestDir
   Returns True if all security checks pass, with the combination of ADestDir
   and AFilename in AResultingPath.
   and AFilename in AResultingPath.
   ADestDir is assumed to be normalized already and have a trailing backslash.
   ADestDir is assumed to be normalized already and have a trailing backslash.
-  AFilename may be a file or directory name. }
+  AFilename may be a file or directory name.
+  If ADestDir is empty then only AFilename is checked. }
 begin
 begin
   { - Don't allow empty names
   { - Don't allow empty names
     - Don't allow forward slashes or repeated slashes
     - Don't allow forward slashes or repeated slashes
@@ -624,21 +626,30 @@ begin
      not PathIsRooted(AFilename) and
      not PathIsRooted(AFilename) and
      not PathCharIsSlash(AFilename[High(AFilename)]) and
      not PathCharIsSlash(AFilename[High(AFilename)]) and
      not PathHasInvalidCharacters(AFilename, False) then begin
      not PathHasInvalidCharacters(AFilename, False) then begin
-    { Our validity checks passed. Now pass the combined path to PathExpand
-      (GetFullPathName) to see if it thinks the path needs normalization.
-      If the returned path isn't exactly what was passed in, then consider
-      the name invalid.
-      One way that can happen is if the path ends in an MS-DOS device name:
-      PathExpand('c:\path\NUL') returns '\\.\NUL'. Obviously we don't want
-      devices being opened, so that must be rejected. }
-    var CombinedPath := ADestDir + AFilename;
-    var TestExpandedPath: String;
-    if PathExpand(CombinedPath, TestExpandedPath) and
-       (CombinedPath = TestExpandedPath) then begin
-      AResultingPath := CombinedPath;
+    if ADestDir <> '' then begin
+      { Our validity checks passed. Now pass the combined path to PathExpand
+        (GetFullPathName) to see if it thinks the path needs normalization.
+        If the returned path isn't exactly what was passed in, then consider
+        the name invalid.
+        One way that can happen is if the path ends in an MS-DOS device name:
+        PathExpand('c:\path\NUL') returns '\\.\NUL'. Obviously we don't want
+        devices being opened, so that must be rejected. }
+      var CombinedPath := ADestDir + AFilename;
+      var TestExpandedPath: String;
+      if PathExpand(CombinedPath, TestExpandedPath) and
+         (CombinedPath = TestExpandedPath) then begin
+        AResultingPath := CombinedPath;
+        Result := True;
+      end;
+    end else
       Result := True;
       Result := True;
-    end;
   end;
   end;
 end;
 end;
 
 
+function ValidateAndCombinePath(const ADestDir, AFilename: String): Boolean;
+begin
+  var ResultingPath: String;
+  Result := ValidateAndCombinePath(ADestDir, AFilename, ResultingPath);
+end;
+
 end.
 end.

+ 7 - 4
Projects/Src/Compression.SevenZipDLLDecoder.pas

@@ -31,7 +31,7 @@ procedure ExtractArchiveRedir(const DisableFsRedir: Boolean;
 type
 type
   TArchiveFindHandle = type Cardinal;
   TArchiveFindHandle = type Cardinal;
 function ArchiveFindFirstFileRedir(const DisableFsRedir: Boolean;
 function ArchiveFindFirstFileRedir(const DisableFsRedir: Boolean;
-  const ArchiveFilename, Password: String; const RecurseSubDirs: Boolean;
+  const ArchiveFilename, DestDir, Password: String; const RecurseSubDirs: Boolean;
   out FindFileData: TWin32FindData): TArchiveFindHandle;
   out FindFileData: TWin32FindData): TArchiveFindHandle;
 function ArchiveFindNextFile(const FindFile: TArchiveFindHandle; out FindFileData: TWin32FindData): Boolean;
 function ArchiveFindNextFile(const FindFile: TArchiveFindHandle; out FindFileData: TWin32FindData): Boolean;
 function ArchiveFindClose(const FindFile: TArchiveFindHandle): Boolean;
 function ArchiveFindClose(const FindFile: TArchiveFindHandle): Boolean;
@@ -782,7 +782,7 @@ end;
 type
 type
   TArchiveFindState = record
   TArchiveFindState = record
     InArchive: IInArchive;
     InArchive: IInArchive;
-    ExtractedArchiveName: String;
+    ExpandedDestDir, ExtractedArchiveName: String;
     RecurseSubDirs: Boolean;
     RecurseSubDirs: Boolean;
     currentIndex, numItems: UInt32;
     currentIndex, numItems: UInt32;
     function GetInitialCurrentFindData(out FindData: TWin32FindData): Boolean;
     function GetInitialCurrentFindData(out FindData: TWin32FindData): Boolean;
@@ -798,7 +798,8 @@ function TArchiveFindState.GetInitialCurrentFindData(out FindData: TWin32FindDat
 
 
   function SkipFile(const Path: String; const IsDir: Boolean): Boolean;
   function SkipFile(const Path: String; const IsDir: Boolean): Boolean;
   begin
   begin
-    Result := not RecurseSubDirs and (IsDir or (PathPos('\', Path) <> 0));
+    Result := (not RecurseSubDirs and (IsDir or (PathPos('\', Path) <> 0))) or
+              not ValidateAndCombinePath(ExpandedDestDir, Path);
   end;
   end;
 
 
 begin
 begin
@@ -836,7 +837,7 @@ begin
 end;
 end;
 
 
 function ArchiveFindFirstFileRedir(const DisableFsRedir: Boolean;
 function ArchiveFindFirstFileRedir(const DisableFsRedir: Boolean;
-  const ArchiveFilename, Password: String; const RecurseSubDirs: Boolean;
+  const ArchiveFilename, DestDir, Password: String; const RecurseSubDirs: Boolean;
   out FindFileData: TWin32FindData): TArchiveFindHandle;
   out FindFileData: TWin32FindData): TArchiveFindHandle;
 begin
 begin
   if ArchiveFileName = '' then
   if ArchiveFileName = '' then
@@ -850,6 +851,8 @@ begin
     State.InArchive := OpenArchiveRedir(DisableFsRedir, ArchiveFilename, Password, clsid);
     State.InArchive := OpenArchiveRedir(DisableFsRedir, ArchiveFilename, Password, clsid);
     if State.InArchive.GetNumberOfItems(State.numItems) <> S_OK then
     if State.InArchive.GetNumberOfItems(State.numItems) <> S_OK then
       SevenZipError('Cannot get number of items', '-3');
       SevenZipError('Cannot get number of items', '-3');
+    if DestDir <> '' then
+      State.ExpandedDestDir := AddBackslash(PathExpand(DestDir));
     State.ExtractedArchiveName := PathExtractName(ArchiveFilename);
     State.ExtractedArchiveName := PathExtractName(ArchiveFilename);
     State.RecurseSubDirs := RecurseSubDirs;
     State.RecurseSubDirs := RecurseSubDirs;
 
 

+ 7 - 5
Projects/Src/Setup.Install.pas

@@ -1901,10 +1901,14 @@ var
 
 
       Result := False;
       Result := False;
 
 
+      if foCustomDestName in CurFile^.Options then
+        InternalError('Unexpected custom DestName');
+      const DestDir = ExpandConst(CurFile^.DestName);
+
       var FindData: TWin32FindData;
       var FindData: TWin32FindData;
       var ArchiveIndex := 0;
       var ArchiveIndex := 0;
-      var H := ArchiveFindFirstFileRedir(DisableFsRedir, ArchiveFilename, Password,
-        foRecurseSubDirsExternal in CurFile^.Options, FindData);
+      var H := ArchiveFindFirstFileRedir(DisableFsRedir, ArchiveFilename, DestDir,
+        Password, foRecurseSubDirsExternal in CurFile^.Options, FindData);
       if H <> INVALID_HANDLE_VALUE then begin
       if H <> INVALID_HANDLE_VALUE then begin
         try
         try
           repeat
           repeat
@@ -1915,9 +1919,7 @@ var
 
 
               Result := True;
               Result := True;
               var SourceFile := ArchiveIndex.ToString; {!!!}
               var SourceFile := ArchiveIndex.ToString; {!!!}
-              if foCustomDestName in CurFile^.Options then
-                InternalError('Unexpected custom DestName');
-              const DestName = ExpandConst(CurFile^.DestName) + FindData.cFileName;
+              const DestName = DestDir + FindData.cFileName;
               var Size: Integer64;
               var Size: Integer64;
               Size.Hi := FindData.nFileSizeHigh;
               Size.Hi := FindData.nFileSizeHigh;
               Size.Lo := FindData.nFileSizeLow;
               Size.Lo := FindData.nFileSizeLow;

+ 9 - 7
Projects/Src/Setup.MainFunc.pas

@@ -1809,9 +1809,13 @@ function EnumFiles(const EnumFilesProc: TEnumFilesProc;
     { See above }
     { See above }
     Result := True;
     Result := True;
 
 
+    if foCustomDestName in CurFile^.Options then
+      InternalError('Unexpected custom DestName');
+    const DestDir = ExpandConst(CurFile^.DestName);
+
     var FindData: TWin32FindData;
     var FindData: TWin32FindData;
-    var H := ArchiveFindFirstFileRedir(DisableFsRedir, ArchiveFilename, Password,
-      foRecurseSubDirsExternal in CurFile^.Options, FindData);
+    var H := ArchiveFindFirstFileRedir(DisableFsRedir, ArchiveFilename, DestDir,
+      Password, foRecurseSubDirsExternal in CurFile^.Options, FindData);
     if H <> INVALID_HANDLE_VALUE then begin
     if H <> INVALID_HANDLE_VALUE then begin
       try
       try
         repeat
         repeat
@@ -1820,9 +1824,7 @@ function EnumFiles(const EnumFilesProc: TEnumFilesProc;
             if IsExcluded(FindData.cFileName, Excludes) then
             if IsExcluded(FindData.cFileName, Excludes) then
               Continue;
               Continue;
 
 
-            if foCustomDestName in CurFile^.Options then
-              InternalError('Unexpected custom DestName');
-            const DestName = ExpandConst(CurFile^.DestName) + FindData.cFileName;
+            const DestName = DestDir + FindData.cFileName;
             if not EnumFilesProc(DisableFsRedir, DestName, Param) then
             if not EnumFilesProc(DisableFsRedir, DestName, Param) then
               Exit(False);
               Exit(False);
           end;
           end;
@@ -2811,8 +2813,8 @@ var
     Result.Lo := 0;
     Result.Lo := 0;
 
 
     var FindData: TWin32FindData;
     var FindData: TWin32FindData;
-    var H := ArchiveFindFirstFileRedir(DisableFsRedir, ArchiveFilename, Password,
-      RecurseSubDirs, FindData);
+    var H := ArchiveFindFirstFileRedir(DisableFsRedir, ArchiveFilename, '',
+      Password, RecurseSubDirs, FindData);
     if H <> INVALID_HANDLE_VALUE then begin
     if H <> INVALID_HANDLE_VALUE then begin
       try
       try
         repeat
         repeat