Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

Isn't it dangerous to use the Longint count with the Int64 size in Stream.read?

I was examining the TMemoryStream class and found the following routine:

procedure TMemoryStream.LoadFromStream(Stream: TStream);
var
  Count: Longint;
begin
  Stream.Position := 0;
  Count := Stream.Size; // <-- assigning Int64 to Longint
  SetSize(Count);
  if Count <> 0 then Stream.ReadBuffer(FMemory^, Count);
end;

I have seen this pattern a lot where an Int64 is assigned to a Longint.

My understanding is that Longint is four bytes and Int64 is eight bytes in both 32-bit and 64-bit Windows, so if my file size is $1 FFFF FFFF == 8589934591 == 8 GB then this routine will simply fail to read because the final count will be $ FFFF FFFF == -1.

I do not understand how this is allowed and maybe not taken into consideration (maybe not many people are trying to read an 8+ GB file).

like image 853
Nasreddine Galfout Avatar asked Dec 31 '17 01:12

Nasreddine Galfout


1 Answers

I logged a ticket for this and it has apparently been fixed in Tokyo 10.2. This is an issue for 64 bit compilation.

https://quality.embarcadero.com/browse/RSP-19094

There are problems with large (>2GB) files in both TCustomMemoryStream and TMemoryStream. In TMemoryStream the issues are simple as the local variables need to be declared as NativeInt instead of LongInt's and Capacity needs to be changed to an NativeInt. In TCustomMemoryStream they are more subtle because both TCustomMemoryStream.Read methods assign the result of an Int64 - Int64 calculation directly to a LongInt. This will overflow even when the result of this calculation isn't larger than a LongInt.

If you want to fix this in Seattle then you will need to either do a code hook, replace the System.Classes unit or roll out your own replacement class for TMemoryStream. Bear in mind that for the last option, you will need to also replace TBytesStream and TStringStream because these descend from TMemoryStream.

The other problem with the the last option is that third party components won't have your "fixes". For us, we only had a couple of places that needed to work with files larger than 2GB so we switched those across.

The fixes for TCustomMemoryStream.Read (must be to both methods) will look something like this:

function TCustomMemoryStream.Read(var Buffer; Count: Longint): Longint;
{ These 2 lines are new }
var
  remaining: Int64;
begin
  if (FPosition >= 0) and (Count >= 0) then
  begin
    remaining{Result} := FSize - FPosition;
    if remaining{Result} > 0 then
    begin
      if remaining{Result} > Count then 
        Result := Count
      else
        Result := remaining;
      Move((PByte(FMemory) + FPosition)^, Buffer, Result);
      Inc(FPosition, Result);
      Exit;
    end;
  end;
  Result := 0;
end;
like image 169
Graymatter Avatar answered Sep 18 '22 13:09

Graymatter