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

Range Check Error in TDECCipherModes.EncodeCBCx() #47

Open
rfoersom opened this issue Sep 10, 2022 · 5 comments
Open

Range Check Error in TDECCipherModes.EncodeCBCx() #47

rfoersom opened this issue Sep 10, 2022 · 5 comments
Assignees

Comments

@rfoersom
Copy link

rfoersom commented Sep 10, 2022

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
2. 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
Copy link
Owner

MHumm commented Sep 11, 2022

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
Copy link
Owner

MHumm commented Oct 26, 2023

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
Copy link
Owner

MHumm commented Dec 17, 2023

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?

1 similar comment
@MHumm
Copy link
Owner

MHumm commented Feb 9, 2024

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
Copy link
Owner

MHumm commented May 4, 2024

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants