MHumm / DelphiEncryptionCompendium

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

Range Check Error in TDECCipherModes.EncodeCBCx() #47

Open rfoersom opened 2 years ago

rfoersom commented 2 years ago

Describe the bug In DEC (6.4.1) when using block cipher, e.g. AES or DES, I encounter range check error exception in DECCipherModes.pas (2022-02-27), TDECCipherModes.EncodeCBCx() when encrypting blocks larger than 32 KiB. It happens because the block cipher type for Source and Dest is PByteArray. PByteArray is defined in Delphi RTL System.SysUtils (Delphi 10.2, 10.3) as:

type
...
  PByteArray = ^TByteArray;
  TByteArray = array[0..32767] of Byte;

To Reproduce 1, Enable range check error in compiler options

  1. Call a function like this:

    function AESEncryptSomething(): TBytes;  
    begin
    SetLength(SouceTab, 256*1024);  // Size >32 KiB
    
    AESCipher:=TCipher_AES.Create;
    AESCipher.Init(AESKey, IV, 0);  // Key and InitVector
    AESCipher.Mode:=cmCBCx;
    Result:=AESCipher.EncodeBytes(SourceTab);  // Range check error!
    FreeAndNil(AESCipher);
    end;

Expected and actual behavior As an user of the DEC lib, your work around method could be to disable range checking for your whole application in the Delphi compiler settings. However that is not a good option.

Alternative work around is in DECCipherModes.pas just after implementation keyword to disable range check for rest of unit. Do:

implementation
{$R-}

To resolve this problem I recommend that DEC lib should rather define and use its own byte data block type, something like:

TDECByteArray = array[0..MaxInt-1] of Byte;

MHumm commented 2 years ago

Ok, I understand the problem you report and I think this kind of issue will be present in other areas like hash calculations as well. I also agree that it should be fixed (currently my time is a bit limited, but I want to find more time for this again). I'd like to check first if Delphi and FPC do bring a suitable replacement type with them before creating my own and I think we need to declare a pointer type for this as well as I think pointers are often used in DEC. We also need to investigate if such a modification brings any drawback (I don't hope so and don't think so, but spending a few minutes on that might be well spent).

Question: if I implemented the modification, would you have valid test data we could use to verify that everything works properly with such bigger array sizes?

MHumm commented 11 months ago

This should be fixed in development branch now. Can you please check if that's ok for you? I could close the issue then.

MHumm commented 9 months ago

Did you already find the time my fixed version in development branch actually fixed your problem? Would it be possible to provide at least one test data set so I can add a unit test for this one?

MHumm commented 7 months ago

Did you already find the time my fixed version in development branch actually fixed your problem? Would it be possible to provide at least one test data set so I can add a unit test for this one?

MHumm commented 5 months ago

Unless I get a negative test report from you within the next few weeks I'll close this one as fixed, as I have made changes to the code.