Harden additional CloseHandle calls against bad inputs

Relates to #509

#509 reports intermittent crashes with PhotoDemon's search function.  Crashes sometimes occur with the error 0xc000041d .  This may be caused by attempting to close null or invalid object handles.

PD shouldn't be doing this, but this commit double-checks all handle values before closing them.  (Sometimes these checks are redundant because embedded functions safely check for null handle state before returning, but better safe than sorry...)

Classes/pdFSO.cls

@@ -218,9 +218,14 @@ Friend Function FileAppendBinaryData(ByVal srcPointer As Long, ByVal srcLengthIn
     Dim hFile As Long
     If Me.FileCreateAppendHandle(dstFilename, hFile) Then
 
-        'Write the byte array and exit!
-        FileAppendBinaryData = Me.FileWriteData(hFile, srcPointer, srcLengthInBytes)
-        Me.FileCloseHandle hFile
+        'Failsafe only
+        If (hFile <> 0) Then
+
+            'Write the byte array and exit!
+            FileAppendBinaryData = Me.FileWriteData(hFile, srcPointer, srcLengthInBytes)
+            Me.FileCloseHandle hFile
+
+        End If
 
     'hFile should technically never be zero, FYI.  An invalid handle value may be returned for generic filesystem errors,
     ' but if a previous "file exists and is writable" check passed, that outcome is extremely unlikely.
@@ -251,10 +256,15 @@ Friend Function FileAppendText(ByVal srcString As String, ByRef dstFilename As S
     Dim hFile As Long
     If Me.FileCreateAppendHandle(dstFilename, hFile) Then
 
-        'Write the byte array and exit!
-        Me.FileWriteData hFile, VarPtr(fileBytes(0)), lenFileBytes
-        Me.FileCloseHandle hFile
-        FileAppendText = True
+        'Failsafe only
+        If (hFile <> 0) Then
+
+            'Write the byte array and exit!
+            Me.FileWriteData hFile, VarPtr(fileBytes(0)), lenFileBytes
+            Me.FileCloseHandle hFile
+            FileAppendText = True
+
+        End If
 
     'hFile should technically never be zero, FYI.  An invalid handle value may be returned for generic filesystem errors,
     ' but if a previous "does file exist" check passed, that outcome is extremely unlikely.
@@ -272,10 +282,15 @@ End Function
 'Close an open file handle.  (Returns TRUE if successful.)
 Friend Function FileCloseHandle(ByRef srcHandle As Long) As Boolean
 
-    If (srcHandle <> 0) Then
+    If (srcHandle <> 0) And (srcHandle <> INVALID_HANDLE_VALUE) Then
         FileCloseHandle = (CloseHandle(srcHandle) <> 0)
     Else
-        FSOError "FileCloseHandle", "was passed an empty handle.  Ask yourself why this handle was already closed!"
+        If (srcHandle = 0) Then
+            FSOError "FileCloseHandle", "empty handle."
+        Else
+            FSOError "FileCloseHandle", "invalid handle."
+            FileCloseHandle = True
+        End If
     End If
 
     'As a convenience to the caller, manually reset the handle to 0.  (This is why it's passed ByRef.)
@@ -433,6 +448,9 @@ Friend Function FileCreateFromPtr(ByVal ptrSrc As Long, ByVal dataLength As Long
         createSuccess = FileCreateHandle(pathToFile, hFile, True, True, OptimizeSequentialAccess)
     End If
 
+    'Failsafe only; successful returns should always provide a non-zero hFile
+    createSuccess = createSuccess And (hFile <> 0)
+
     If createSuccess Then
         FileCreateFromPtr = FileWriteData(hFile, ptrSrc, dataLength)
         CloseHandle hFile
@@ -718,28 +736,34 @@ Friend Function FileGetTimeAsCurrency(ByRef srcFile As String, Optional ByVal ty
     Dim hFile As Long
     If FileCreateHandle(srcFile, hFile, True) Then
 
-        Dim createTime As WAPI_FILETIME, accessTime As WAPI_FILETIME, writeTime As WAPI_FILETIME
-        If (GetFileTime(hFile, createTime, accessTime, writeTime) <> 0) Then
-
-            Dim srcTime As WAPI_FILETIME
+        'Failsafe only
+        If (hFile <> 0) Then
+
+            Dim createTime As WAPI_FILETIME, accessTime As WAPI_FILETIME, writeTime As WAPI_FILETIME
+            If (GetFileTime(hFile, createTime, accessTime, writeTime) <> 0) Then
+
+                Dim srcTime As WAPI_FILETIME
+
+                If (typeOfTime = PDFT_AccessTime) Then
+                    srcTime = accessTime
+                ElseIf (typeOfTime = PDFT_WriteTime) Then
+                    srcTime = writeTime
+                Else
+                    srcTime = createTime
+                End If
+
+                'At present, PD only needs file times for checksum purposes.  In the future, it would be nice to add
+                ' comparison functions, but for now, you have to deal with times being returned as currency.
+                CopyMemoryStrict VarPtr(FileGetTimeAsCurrency), VarPtr(srcTime.dwHighDateTime), 4&
+                CopyMemoryStrict VarPtr(FileGetTimeAsCurrency) + 4, VarPtr(srcTime.dwLowDateTime), 4&
 
-            If (typeOfTime = PDFT_AccessTime) Then
-                srcTime = accessTime
-            ElseIf (typeOfTime = PDFT_WriteTime) Then
-                srcTime = writeTime
-            Else
-                srcTime = createTime
             End If
 
-            'At present, PD only needs file times for checksum purposes.  In the future, it would be nice to add
-            ' comparison functions, but for now, you have to deal with times being returned as currency.
-            CopyMemoryStrict VarPtr(FileGetTimeAsCurrency), VarPtr(srcTime.dwHighDateTime), 4&
-            CopyMemoryStrict VarPtr(FileGetTimeAsCurrency) + 4, VarPtr(srcTime.dwLowDateTime), 4&
-
+            CloseHandle hFile
+
+        '/hFile != 0
         End If
 
-        CloseHandle hFile
-
     End If
 
 End Function
@@ -850,8 +874,10 @@ Friend Function FileLenW(ByRef srcPath As String) As Long
 
     Dim hFile As Long
     If Me.FileCreateHandle(srcPath, hFile, True) Then
-        FileLenW = CLng(FileLenW_Large(hFile))
-        Me.FileCloseHandle hFile
+        If (hFile <> 0) Then
+            FileLenW = CLng(FileLenW_Large(hFile))
+            Me.FileCloseHandle hFile
+        End If
     Else
         FileLenW = 0
     End If
@@ -894,36 +920,42 @@ Friend Function FileLoadAsByteArray(ByRef srcFile As String, ByRef dstArray() As
         Dim hFile As Long
         If FileCreateHandle(srcFile, hFile, True, False, OptimizeSequentialAccess) Then
 
-            'Prep the destination buffer.  (Note that we check buffer size in advance; there's no reason to resize it
-            ' if we're just going to overwrite it with new data!)
-            Dim lenOfFile As Long
-            lenOfFile = CLng(FileLenW_Large(hFile))
-
-            'Only proceed if the file is not zero-length
-            If (lenOfFile > 0) Then
+            'Failsafe only
+            If (hFile <> 0) Then
 
-                If VBHacks.IsArrayInitialized(dstArray) Then
-                    If (LBound(dstArray) <> 0) Or (UBound(dstArray) <> lenOfFile - 1) Then ReDim dstArray(0 To lenOfFile - 1) As Byte
-                Else
-                    ReDim dstArray(0 To lenOfFile - 1) As Byte
-                End If
+                'Prep the destination buffer.  (Note that we check buffer size in advance; there's no reason to resize it
+                ' if we're just going to overwrite it with new data!)
+                Dim lenOfFile As Long
+                lenOfFile = CLng(FileLenW_Large(hFile))
 
-                'Retrieve the full file contents
-                Dim tmpVerify As Long
-                If (ReadFile(hFile, VarPtr(dstArray(0)), lenOfFile, tmpVerify, 0&) <> 0) Then
-                    FileLoadAsByteArray = True
+                'Only proceed if the file is not zero-length
+                If (lenOfFile > 0) Then
+
+                    If VBHacks.IsArrayInitialized(dstArray) Then
+                        If (LBound(dstArray) <> 0) Or (UBound(dstArray) <> lenOfFile - 1) Then ReDim dstArray(0 To lenOfFile - 1) As Byte
+                    Else
+                        ReDim dstArray(0 To lenOfFile - 1) As Byte
+                    End If
+
+                    'Retrieve the full file contents
+                    Dim tmpVerify As Long
+                    If (ReadFile(hFile, VarPtr(dstArray(0)), lenOfFile, tmpVerify, 0&) <> 0) Then
+                        FileLoadAsByteArray = True
+                    Else
+                        FSOError "FileLoadAsByteArray", "failure when calling ReadFile on " & srcFile & ". Abandoning load."
+                    End If
+
                 Else
-                    FSOError "FileLoadAsByteArray", "failure when calling ReadFile on " & srcFile & ". Abandoning load."
+                    FSOError "FileLoadAsByteArray", "source file is zero-length: " & srcFile
+                    ReDim dstArray(0) As Byte
+                    FileLoadAsByteArray = False
                 End If
 
-            Else
-                FSOError "FileLoadAsByteArray", "source file is zero-length: " & srcFile
-                ReDim dstArray(0) As Byte
-                FileLoadAsByteArray = False
+                CloseHandle hFile
+
+            '/hFile != 0
             End If
-
-            CloseHandle hFile
-
+
         Else
             FSOError "FileLoadAsByteArray", "failed to create a valid handle for " & srcFile & ". Abandoning load."
         End If
@@ -1288,20 +1320,26 @@ Friend Function FileSaveAsText(ByRef srcString As String, ByRef dstFilename As S
         'Open the file
         Dim hFile As Long
         If Me.FileCreateHandle(dstFilename, hFile, False, True, OptimizeSequentialAccess) Then
-
-            'If requested, write a BOM first.  This is desirable for PD's internal files,
-            ' as it makes it very quick to identify new UTF-8 ones vs old Windows-1252 ones.
-            If useUTF8_BOM Then
-                Dim bomMarker(0 To 2) As Byte
-                bomMarker(0) = &HEF: bomMarker(1) = &HBB: bomMarker(2) = &HBF
-                Me.FileWriteData hFile, VarPtr(bomMarker(0)), 3
-            End If
-
-            'Write the byte array and exit!
-            Me.FileWriteData hFile, VarPtr(fileBytes(0)), lenFileBytes
-            Me.FileCloseHandle hFile
-            FileSaveAsText = True
 
+            'Failsafe only
+            If (hFile <> 0) Then
+
+                'If requested, write a BOM first.  This is desirable for PD's internal files,
+                ' as it makes it very quick to identify new UTF-8 ones vs old Windows-1252 ones.
+                If useUTF8_BOM Then
+                    Dim bomMarker(0 To 2) As Byte
+                    bomMarker(0) = &HEF: bomMarker(1) = &HBB: bomMarker(2) = &HBF
+                    Me.FileWriteData hFile, VarPtr(bomMarker(0)), 3
+                End If
+
+                'Write the byte array and exit!
+                Me.FileWriteData hFile, VarPtr(fileBytes(0)), lenFileBytes
+                Me.FileCloseHandle hFile
+                FileSaveAsText = True
+
+            '/hFile != 0
+            End If
+
         Else
             FSOError "FileSaveAsText", "could not create file handle"
         End If

Modules/OS.bas

@@ -465,7 +465,7 @@ Private Function AppProcessID() As Long
             End If
 
             'Regardless of outcome, close the ToolHelp enumerator when we're done
-            CloseHandle hSnapShot
+            If (hSnapShot <> 0) Then CloseHandle hSnapShot
 
         Else
             m_AppProcID = 0

Modules/Web.bas

@@ -44,15 +44,6 @@ End Enum
 #End If
 
 'Download code from some random dialogs (e.g. "download image") needs to be moved here! TODO
-'Private Declare Function HttpQueryInfoW Lib "wininet" (ByVal hHttpRequest As Long, ByVal lInfoLevel As Long, ByVal ptrSBuffer As Long, ByRef lBufferLength As Long, ByRef lIndex As Long) As Long
-'Private Declare Function InternetCloseHandle Lib "wininet" (ByVal hInet As Long) As Long
-'Private Declare Function InternetOpenW Lib "wininet" (ByVal lpszAgent As Long, ByVal dwAccessType As Long, ByVal lpszProxyName As Long, ByVal lpszProxyBypass As Long, ByVal dwFlags As Long) As Long
-'Private Declare Function InternetOpenUrlW Lib "wininet" (ByVal hInternetSession As Long, ByVal lpszUrl As Long, ByVal lpszHeaders As Long, ByVal dwHeadersLength As Long, ByVal dwFlags As Long, ByVal dwContext As Long) As Long
-'Private Declare Function InternetReadFile Lib "wininet" (ByVal hFile As Long, ByVal ptrToBuffer As Long, ByVal dwNumberOfBytesToRead As Long, ByRef lNumberOfBytesRead As Long) As Long
-'
-'Private Const HTTP_QUERY_CONTENT_LENGTH As Long = 5
-'Private Const INTERNET_OPEN_TYPE_PRECONFIG As Long = 0
-'Private Const INTERNET_FLAG_RELOAD As Long = &H80000000
 
 'Open a string as a hyperlink in the user's default browser
 Public Sub OpenURL(ByRef targetURL As String)
@@ -179,7 +170,7 @@ Public Function DownloadURLToTempFile(ByRef URL As String, Optional ByVal suppre
 
     If (hUrl = 0) Then
         If (Not suppressErrorMsgs) Then PDMsgBox "PhotoDemon could not reach the target URL (%1).  If the problem persists, try downloading the file manually using your Internet browser.", vbExclamation Or vbOKOnly, "Error", URL
-        If hInternetSession Then InternetCloseHandle hInternetSession
+        If (hInternetSession <> 0) Then InternetCloseHandle hInternetSession
         DownloadURLToTempFile = vbNullString
         Screen.MousePointer = 0
         Exit Function

PhotoDemon.vbp

@@ -1,6 +1,6 @@
 Type=Exe
-Reference=*\G{50A7E9B0-70EF-11D1-B75A-00A0C90564FE}#1.0#0#..\..\Windows\SysWOW64\shell32.dll#Microsoft Shell Controls And Automation
-Reference=*\G{00020430-0000-0000-C000-000000000046}#2.0#0#..\..\Windows\SysWOW64\stdole2.tlb#OLE Automation
+Reference=*\G{50A7E9B0-70EF-11D1-B75A-00A0C90564FE}#1.0#0#\\?\C:\Windows\SysWOW64\shell32.dll#Microsoft Shell Controls And Automation
+Reference=*\G{00020430-0000-0000-C000-000000000046}#2.0#0#\\?\C:\Windows\SysWOW64\stdole2.tlb#OLE Automation
 Module=PDMain; Modules\Main.bas
 Module=Public_Constants; Modules\PublicConstants.bas
 Module=Public_EnumsAndTypes; Modules\PublicEnumsAndTypes.bas
@@ -520,7 +520,7 @@ Description="PhotoDemon Photo Editor"
 CompatibleMode="0"
 MajorVer=9
 MinorVer=1
-RevisionVer=243
+RevisionVer=245
 AutoIncrementVer=1
 ServerSupportFiles=0
 VersionComments="Copyright 2000-2023 Tanner Helland - photodemon.org"