MHumm / DelphiEncryptionCompendium

Cryptographic library for Embarcadero Delphi and potentially for FPC as well
Apache License 2.0
257 stars 67 forks source link

Integer Overflow when hashing a large TFilestream #59

Closed iofly closed 9 months ago

iofly commented 1 year ago

I was trying to hash large TFileStream (all except the first 10MB of the file). I'm using a 900MB file in testing. When I call Hash.CalcStream(...) on the TFileStream I get an Integer Overflow exception (EIntOverflow).

Tested with THash_SHA256 and THash_SHA512

To Reproduce Run the following:

uses DECHash;

procedure TfrmTest.Button1Click(Sender: TObject);
var
  fsIn: TFileStream;
  b: TBytes;
begin

  fsIn:=TFileStream.Create('900MBfile.dat', fmOpenRead);
  try
    fsIn.Seek(10000000, soFromBeginning); //skip the first 10 MB
    b:=HashStream_SHA256(fsIn);
  finally
    fsIn.Free;
  end;

  System.IOUtils.TFile.WriteAllBytes('hash.bytes', b);
end;

function TfrmTest.HashStream_SHA256(s: TFileStream): TBytes;
var
  Hash : THash_SHA256;
  HashResult: TBytes;
begin
  Hash:=THash_SHA256.Create;
  try
    Hash.CalcStream(s, s.Size - s.Position, HashResult);
  finally
    Hash.Free;
  end;

  result:=HashResult;

end;

Expected and actual behavior Expected HashResult to be populated with the hash bytes and no excpetions raised. Actual behavior: An exception is thrown when hashing large files using CalcStream(). When I use a small file (around 15MB) there is no exception and it functions as normal.

M0nsieurT commented 1 year ago

Hi,

Depending on your Delphi version, you will need to define integer overflow, I got the same issue with Delphi 11 but no error on 10 : https://docwiki.embarcadero.com/RADStudio/Alexandria/en/Overflow_checking_(Delphi)

MHumm commented 1 year ago

I don't know (yet) how a TFileStream or TStream works internally. If it holds all data in RAM that might explain it, but I'm not sure about this yet.

MHumm commented 11 months ago

Ok, I took your code now and created a test project. I used a > 600 MB file and it shows your problem. It is in TDECHash.Increment8(var Value; Add: UInt32);

I added a possible fix now, see development branch. But I'm not sure yet this is what we want to have. A discussion about this was started on German Delphipraxis.net forums now. If they say my proposed solution is ok, we'll take it and can close this report. My proposed fix at least runs your test case fine and doesn't create any new problems with the existing unit tests.

MHumm commented 11 months ago

Fixed in development branch now.

MHumm commented 9 months ago

Since this is fixed in development branch and nobody objected the way it was fixed since before Christmas I close this issue now.