Access violation if GCM mode is set after cipher is initialized

[danielmarschall]

To Reproduce

1. Open demo project Cipher_Console
2. Change Cipher.Mode to cmGCM
3. Run the code. You receive EAccessViolation

---

Usually, if you change the cipher mode after the cipher has been initialized, then TDECCipher.SetMode should call Done on the cipher and re-initialize it.

GCM Encode requires that TGCM.Init is called and has set FEncryptionMethod := EncryptionMethod. However, if Ciper.Mode is set after the cipher has been initialized, then TGCM is created but TGCM.Init has not been called.

procedure TDECCipherModes.InitMode;
begin
  if FMode = TCipherMode.cmGCM then
  begin
    if Context.BlockSize = 16 then
      FGCM := TGCM.Create
    else
      // GCM requires a cipher with 128 bit block size
      raise EDECCipherException.CreateResFmt(@sInvalidBlockSize,
                                             [128, GetEnumName(TypeInfo(TCipherMode),
                                             Integer(FMode))]);
  end
  else
    if Assigned(FGCM) then
      FreeAndNil(FGCM);
end;

The program flow if Mode has been set before init:

1. TDECCipher.Init()
2. TDECCipher.Init()
3. TDECCipherModes.OnAfterInitVectorInitialized ( calls FGCM.Init(self.DoEncode, OriginalInitVector) )
4. TGCM.Init

The program flow if Mode has been set after init:

1. TDECCipher.SetMode
2. TDECCipherModes.InitMode
3. TGCM.Create, but not TGCM.Init ( I am not sure where I get OriginalInitVector from )

So... what do we do? We probably need to preserve the InitVector, so we can call TGCM.Init with the correct parameters.

The other solution is to forbid to change the mode after the cipher has been initialized. But this is a code breaking change.

[MHumm]

I understand the issue. If we need to call GCM.Init later on, we need the value of the InitVector. That would require to keep it in memory until everything is finished (ok, changing the mode after starting encryption/decryption should not be node. Not sure at the moment if that will raise some exception).

The question is: how much of a safety issue would that keeping around of the IV be?

[danielmarschall]

Since a lot of time has passed, I am not familar with the code anymore, but if you say that the IV needs to stay in memory, then this could be an attack vector, I guess. For example, if you have a DRM application that is actively streaming content, then you could steal the IV (also the key?) by reading the process memory.

If there is no possibility to fix the issue without introducing this security risk, then I think there should be an Exception saying that you must call X before Y. What do you think?

[MHumm]

I haven't further looked at this one yet, but another approach could be to create an instance of that GCM class directly in the inith method and keep that one around. As far as I remember the user has no method to change the IV after calling init. It would waste some RAM but it should work. We'd only use that instance if somebody really selected this mode.