SetLength TBytes Memory Leak

[hafedh-trimeche]

Delphi: 10.3 Version 26.0.36039.7899

This code generates a memory at the first call to SetLength:

function Mime64Decode(const Encoded:string):TBytes;
var
  InLen  ,
  OutLen : NativeUInt;
  Raw    : RawByteString;
begin
  Raw    := RawByteString(Encoded);
  InLen  := Length(Encoded);
  OutLen := 0;
  if InLen>0 then
  begin
    OutLen := MimeDecodedSize(InLen);
    SetLength(Result,OutLen);
    try
      OutLen := MimeDecode(Raw[1],InLen,Result[0]);
    except
      OutLen := 0;
    end;
  end;
  SetLength(Result,OutLen);
end;

A memory block has been leaked. The size is: 511

This block was allocated by thread 0xA9C, and the stack trace (return addresses) at the time was:
00420A36 [FastMM5.pas][FastMM5][FastMM_DebugGetMem][7718]
00407441 [System.pas][System][@ReallocMem][5022]
0040DB51 [System.pas][System][DynArraySetLength][36046]
0040B09C [System.pas][System][@LStrFromPWCharLen][26213]
0040DC92 [System.pas][System][@DynArraySetLength][36150]
00811251 [uCommon.pas][uCommon][Mime64Decode][3730]
00928475 [uXMLSec.pas][uXMLSec][NodeGetBase64Value][311]
0092AF54 [uXMLSec.pas][uXMLSec][VerifyNode][893]
0092B800 [uXMLSec.pas][uXMLSec][TXMLSec.Verify][1006]
00D434AE [Main.pas][Main][TMainForm.UpdateAttachmentsAndSignatures][298]
00D43EB8 [Main.pas][Main][TMainForm.LoadFile][427]
00D43FBA [Main.pas][Main][TMainForm.BtnOpenClick][443]
00578045 [Vcl.Controls.pas][Vcl.Controls][TControl.Click][7536]
0059B81F [Vcl.StdCtrls.pas][Vcl.StdCtrls][TCustomButton.Click][5470]
0059C335 [Vcl.StdCtrls.pas][Vcl.StdCtrls][TCustomButton.CNCommand][5931]
00577AE9 [Vcl.Controls.pas][Vcl.Controls][TControl.WndProc][7420]
75FD5E7A [Unknown function at GetClassLongW]
75FD60BF [Unknown function at GetClassLongW]
75FD5EBC [Unknown function at GetClassLongW]
75FD5E7A [Unknown function at GetClassLongW]

Best regards.

[pleriche]

Hi,

I don't see anything suspicious in the code snippet, so I believe that something is going wrong after the TBytes result is assigned by the code that is calling Mime64Decode. Perhaps the result is assigned to a field of a record that is later cleared using FillChar, or it is assigned to a field of an object that is never freed (but then you should also get a leak report for the object). I would investigate what happens to the result of the call to Mime64Decode in uXMLSec.pas NodeGetBase64Value.

If you get stuck finding the culprit, you're welcome to e-mail me a compileable test case and I'll look into it for you.

Best regards, Pierre

[hafedh-trimeche]

Hello Pierre,

Thank you for your availability.

Please note that there is no call to FillChar. All calls to FillChar are substituted with InitRecord:

procedure InitRecord(Out Buffer;const Size:Integer;const Value:Byte);
begin
  FillChar(Buffer,Size,Value);
end;

The function where leak is detected:

function NodeGetBase64Value(const node:xmlNodePtr):TBytes;
var
  Buffer    : xmlSecBufferPtr;
  BufferLen : Integer;
begin
  Buffer := xmlSecBufferCreate(0);
  xmlSecBufferBase64NodeContentRead(Buffer,node);
  BufferLen := xmlSecBufferGetSize(Buffer);
  if BufferLen>0 then
  begin
    SetLength(Result,BufferLen);
    Move(xmlSecBufferGetData(Buffer)^,Result[0],BufferLen);
  end
  else Result := nil;
  xmlSecBufferDestroy(Buffer);
end;

Please note that if SetLength(Result,BufferLen) not invoked, no leak generated.

xmlSec functions are imported form libxml2 & libxmlsec C libraries and never caused a leak (the Buffer is destroyed before exiting the function).

Best regards.

[pleriche]

That InitRecord procedure is very dangerous. If the record contains managed fields, like TBytes, then the reference to the payload will be zeroed out, but the payload itself will leak. I believe that is what is happening here.

I think you will find that this InitRecord is called on a record that contains the TBytes that is returned by the calls above.

[hafedh-trimeche]

Please find attached a project which demonstrates that InitRecord never leaks, FillChar does. Leak.zip

[pleriche]

Ah yes, sorry, I missed that the Buffer parameter is marked as "out", so the record gets finalized before InitRecord is called - at least in Delphi code. You don't perhaps call InitRecord from C++ Builder code? I've read somewhere that C++ Builder code treats "out" the same as "var", so this trick won't work there and it will cause the managed content of the record to leak.

If not, then the problem does indeed lie elsewhere in the code. It's very unlikely to be an issue in the memory manager itself (you can confirm that by using the built-in memory manager and setting ReportMemoryLeaksOnShutdown := True), but if you send me a test case I can compile and run then I'll take a look.

[hafedh-trimeche]

The leak is generated by a bad usage of SSL routine. The result of Base64Decode (TBytes=Blob) is passed to d2i_OCSP_RESPONSE as a Pointer not double Pointer PPointer. Bad:

  PBlob := Blob;
  resp  := d2i_OCSP_RESPONSE(nil,@PBlob,Length(Blob));

Correct:

  PBlob := @Blob;
  resp  := d2i_OCSP_RESPONSE(nil,@PBlob,Length(Blob));

Best regards.