Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Potential TPCTemporalFileStream object leak in UNetProtocol.pas: DownloadSafeboxChunks #241

Open
SkybuckFlying opened this issue Jan 27, 2022 · 0 comments

Comments

@SkybuckFlying
Copy link
Contributor

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_',[i])); // *** 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;
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant