entronad / crypto-es

A cryptography algorithms library
Other
272 stars 30 forks source link

Hashing/Decrypting bug #25

Closed grahamlyus closed 1 year ago

grahamlyus commented 2 years ago

Hashing decrypted data seems to fail when data isn't a multiple of 4 bytes (i.e. whole words). This can be fixed by clamping the WordArray, but I'm not sure if this is a bug with the Hashers or the Decryptors. See Jest test below:

import CryptoES from 'crypto-es';

describe('hashing/decrypting bug', () => {
  const data11Bytes = 'hello world';
  const data11BytesHash = CryptoES.SHA256(data11Bytes).toString();

  // clamp will remove the last word and 0 the last byte of word 2
  const decrypted11Bytes = new CryptoES.lib.WordArray(
    [
      1751477356, // 68656C6C
      1864398703, // 6F20776F
      1919706117, // 726C6405
      84215045, // 05050505
    ],
    11,
  );
  const decrypted11BytesClamped = new CryptoES.lib.WordArray(
    [
      1751477356, // 68656C6C
      1864398703, // 6F20776F
      1919706112, // 726C6400
    ],
    11,
  );
  const data12Bytes = 'hello world!';
  const data12BytesHash = CryptoES.SHA256(data12Bytes).toString();

  test('hash of decrypted 11 bytes is wrong without clamping', () => {
    const key = CryptoES.lib.WordArray.random(32);
    const iv = CryptoES.lib.WordArray.random(32);

    const encryptor = CryptoES.algo.AES.createEncryptor(key, {iv});
    const encryptedData = encryptor.finalize(data11Bytes);

    const decryptor = CryptoES.algo.AES.createDecryptor(key, {iv});
    const decryptedData = decryptor.finalize(encryptedData);
    expect(decryptedData).toEqual(decrypted11Bytes);
    const decryptedDataHash = CryptoES.SHA256(decryptedData).toString();

    // FAIL - SHOULD BE EQUAL.
    expect(decryptedDataHash).not.toEqual(data11BytesHash);
  });
  test('hash of decrypted 12 bytes is correct without clamping', () => {
    const key = CryptoES.lib.WordArray.random(32);
    const iv = CryptoES.lib.WordArray.random(32);

    const encryptor = CryptoES.algo.AES.createEncryptor(key, {iv});
    const encryptedData = encryptor.finalize(data12Bytes);

    const decryptor = CryptoES.algo.AES.createDecryptor(key, {iv});
    const decryptedData = decryptor.finalize(encryptedData);
    const decryptedDataHash = CryptoES.SHA256(decryptedData).toString();

    // SUCCESS
    expect(decryptedDataHash).toEqual(data12BytesHash);
  });
  test('hash of decrypted 11 bytes is correct with clamping', () => {
    const key = CryptoES.lib.WordArray.random(32);
    const iv = CryptoES.lib.WordArray.random(32);

    const encryptor = CryptoES.algo.AES.createEncryptor(key, {iv});
    const encryptedData = encryptor.finalize(data11Bytes);

    const decryptor = CryptoES.algo.AES.createDecryptor(key, {iv});
    const decryptedData = decryptor.finalize(encryptedData);

    expect(decryptedData).toEqual(decrypted11Bytes);
    decryptedData.clamp();
    expect(decryptedData).toEqual(decrypted11BytesClamped);

    expect(decryptedData).not.toEqual(decrypted11Bytes);
    const decryptedDataHash = CryptoES.SHA256(decryptedData).toString();

    // SUCCESS
    expect(decryptedDataHash).toEqual(data11BytesHash);
  });
});
entronad commented 1 year ago

This behavior is consistent with CryptoJS. I'm not sure the idea of the original designer, (The maintainer of CryptoJS in not the original author).