Potential TPCTemporalFileStream object leak in UNetProtocol.pas: DownloadSafebox

Started by Skybuck, January 27, 2022, 01:28:45 PM

Previous topic - Next topic

Skybuck

See *** for possible problematic lines.

Destroying of this object is "conditional", depends on return of function, also depends on if exception is raised or not.

Quite sloppy, recommend always destroying the object no matter what the conditional or exception is.

It does not seem the called functions free the object.

  Function DownloadSafeboxChunks(ASafeboxChunks : TPCSafeboxChunks; var ASafeboxLastOperationBlock : TOperationBlock; var errors : String) : Boolean;
  var LDownloadedSafeboxBlocksCount, request_id : Cardinal;
    LreceivedChunk : TStream;
    safeBoxHeader : TPCSafeBoxHeader;
    //errors : String;
    i : Integer;
  Begin
    Result := False;
    ASafeboxChunks.Clear;
    // Will try to download penultimate saved safebox
    LDownloadedSafeboxBlocksCount := ((Connection.FRemoteOperationBlock.block DIV CT_BankToDiskEveryNBlocks)-1) * CT_BankToDiskEveryNBlocks;

    If not Do_GetOperationBlock(LDownloadedSafeboxBlocksCount,5000,ASafeboxLastOperationBlock) then begin
      Connection.DisconnectInvalidClient(false,Format('Cannot obtain operation block %d for downloading safebox',[LDownloadedSafeboxBlocksCount]));
      exit;
    end;
    // New Build 2.1.7 - Check valid operationblock
    If Not TPCSafeBox.IsValidOperationBlock(ASafeboxLastOperationBlock,errors) then begin
      Connection.DisconnectInvalidClient(false,'Invalid operation block at DownloadSafeBox '+TPCOperationsComp.OperationBlockToText(ASafeboxLastOperationBlock)+' errors: '+errors);
      Exit;
    end;
      // Will obtain chunks of 10000 blocks each -> Note: Maximum is CT_MAX_SAFEBOXCHUNK_BLOCKS
      for i:=0 to ((LDownloadedSafeboxBlocksCount-1) DIV 10000) do begin // Bug v3.0.1 and minors
        FNewBlockChainFromClientStatus := Format('Receiving new safebox with %d blocks (step %d/%d) from %s',
          [LDownloadedSafeboxBlocksCount,i+1,((LDownloadedSafeboxBlocksCount-1) DIV 10000)+1,Connection.ClientRemoteAddr]);
        LreceivedChunk := TPCTemporalFileStream.Create(Format('CHUNK_%.3d_',)); // *** OBJECT IS CREATED ***
        if (Not DownloadSafeBoxChunk(LDownloadedSafeboxBlocksCount,ASafeboxLastOperationBlock.initial_safe_box_hash,(i*10000),((i+1)*10000)-1,LreceivedChunk,safeBoxHeader,errors)) then begin
          LreceivedChunk.Free;  // *** OBJECT IS ONLY DESTROYED ON THIS CONDITION ***
          TLog.NewLog(ltError,CT_LogSender,errors);
          Exit;
        end;
        try
          LreceivedChunk.Position := 0;
          ASafeboxChunks.AddChunk( LreceivedChunk );
        Except
          On E:Exception do begin
            errors:= Format('(%s) %s',[E.ClassName,E.Message]);
            Result := false;
            LreceivedChunk.Free; // *** OBJECT IS ONLY DESTROYED ON EXCEPTION ***
            Exit;
          end;
        end;
      end;

      if Not ASafeboxChunks.IsComplete then begin
        errors := 'Safebox Chunks is not complete!';
        Exit;
      end else Result := True;
  end;