7 memory leaks in OverbyteIcsWSocket.pas - with fixes

[Ralf Junker 1]

The patch below suggests fixes for 7 memory leaks in OverbyteIcsWSocket.pas. They surfaced after the OverbyteIcsSslWebServ.dpr sample project was linked against YuOpenSSL. A total of > 10k memory allocations reported not freed, adding up to > 500k bytes of leaked memory.

 

The leaks were easy to spot because YuOpenSSL does not use the OpenSSL DLLs but compiles all OpenSSL code into the application binary. As a side effect, YuOpenSSL uses the Delphi memory manager and by default allows memory trackers to see OpenSSL memory allocations. Otherwise this does normally not happen when the OpenSSL DLLs employ their own memory management. The leaks then sum up in the DLLs' memory space, and out of sight of Delphi's memory trackers.

 

For anyone who would like to test their ICS applications for memory leaks with YuOpenSSL, here is the link: https://www.yunqa.de/delphi/products/openssl/index

Index: OverbyteIcsWSocket.pas
===================================================================
--- OverbyteIcsWSocket.pas	(revision 1492)
+++ OverbyteIcsWSocket.pas	(working copy)
@@ -13933,7 +13933,8 @@
     for I := 0 to Result - 1 do begin
         P := f_OPENSSL_sk_value(CertStack, I);
         if Assigned(P) then  { V8.64 sanity test }
-            Add(f_X509_dup(PX509(P)));
+            // Add(f_X509_dup(PX509(P))); // ralfjunker fix memory leak.
+            Add(PX509(P)); // ralfjunker fix memory leak.
     end;
 end;
 
@@ -14866,7 +14867,8 @@
                          OneErr := String(LastOpenSslErrMsg(FALSE));
                     OneErr := 'Error Cert ' + IntToStr(tot) + ' - ' + OneErr;
                 end;
-            end;
+            // end; ralfjunker fix memory leak.
+            end else f_BIO_free(MemBio); // ralfjunker fix memory leak.
             // ignore private keys and other stuff
 
             if OneErr <> '' then begin
@@ -17369,7 +17371,8 @@
 procedure TX509Base.FreeAndNilX509Inters;         { V8.41 }
 begin
     if Assigned(FX509Inters) then begin
-        f_OPENSSL_sk_free(FX509Inters);
+        // f_OPENSSL_sk_free(FX509Inters); // ralfjunker fix memory leak.
+        f_OPENSSL_sk_pop_free(FX509Inters, @f_X509_free); // ralfjunker fix memory leak.
         FX509Inters := nil;
     end;
 end;
@@ -18334,7 +18337,10 @@
  // pending, search stack for server certificate, might not be first
 
     { first in stack is server certificate }
-        SetX509(PX509(f_OPENSSL_sk_delete(MyStack, 0)));
+        // SetX509(PX509(f_OPENSSL_sk_delete(MyStack, 0))); // ralfjunker fix memory leak.
+        x := PX509(f_OPENSSL_sk_delete(MyStack, 0));        // ralfjunker fix memory leak.
+        SetX509(x);                                         // ralfjunker fix memory leak.
+        f_X509_free(x);                                     // ralfjunker fix memory leak.
 
     { remaineder are intermediates }
         if (TotInfo > 1) and (IncludeInters > croNo) then begin
@@ -19394,7 +19400,8 @@
 begin
     result := '' ;
     if not Assigned(X509) then Exit;
-    pubkey := f_X509_get_pubkey(X509);
+    // pubkey := f_X509_get_pubkey(X509); // ralfjunker fix memory leak.
+    pubkey := f_X509_get0_pubkey(X509);   // ralfjunker fix memory leak.
     Result := GetKeyDesc(pubkey);
 end;
 
@@ -19500,17 +19507,27 @@
 
 {* * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * *}
 procedure TX509Base.LoadIntersFromPemFile(const FileName: String);         { V8.41 }
+var          // ralfjunker fix memory leak.
+  x: PStack; // ralfjunker fix memory leak.
 begin
     InitializeSsl;
-    SetX509Inters(IcsSslLoadStackFromInfoFile(FileName, emCert));
+    // SetX509Inters(IcsSslLoadStackFromInfoFile(FileName, emCert)); // ralfjunker fix memory leak.
+    x := IcsSslLoadStackFromInfoFile(FileName, emCert);              // ralfjunker fix memory leak.
+    SetX509Inters(x);                                                // ralfjunker fix memory leak.
+    f_OPENSSL_sk_free(x);                                            // ralfjunker fix memory leak.
 end;
 
 
 {* * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * *}
 procedure TX509Base.LoadIntersFromString(const Value: String);             { V8.41 }
+var          // ralfjunker fix memory leak.
+  x: PStack; // ralfjunker fix memory leak.
 begin
     InitializeSsl;
-    SetX509Inters(IcsSslLoadStackFromInfoString(Value, emCert));
+    // SetX509Inters(IcsSslLoadStackFromInfoString(Value, emCert)); // ralfjunker fix memory leak.
+    x := IcsSslLoadStackFromInfoString(Value, emCert);              // ralfjunker fix memory leak.
+    SetX509Inters(x);                                               // ralfjunker fix memory leak.
+    f_OPENSSL_sk_free(x);                                           // ralfjunker fix memory leak.
 end;

 

[Angus Robertson 577]

Thanks, will do the changes next week.  In future, can you please add a patch text attachment, I know from experience that trying to use HTML Unicode for patches in Delphi causes corruption.

 

Angus

 

[Angus Robertson 577]

These memory leaks are now fixed in SVN, will be zipped overnight, thanks again. 

 

Angus