Minor bug in RC2 key expansion function

[Martomate]

There seems to be a missing minus sign in the key expansion function for RC2 on this line. The T1 variable should be negated (which is what they do here).

- var TM = 0xff >> (T1 & 0x07);
+ var TM = 0xff >> (-T1 & 0x07);

This bug only affects RC2 if the "effective key bits" (T1) is not a multiple of 8, and since the default is 128 it probably doesn't affect that many users. Notably, CyberChef is not affected (since they use the default value).

I have verified that 0xff >> (-T1 & 0x07) is equivalent to the expression used in the spec, whereas 0xff >> (T1 & 0x07) is not.

[davidlehn]

What's a good way to add a test for this?

[Martomate]

What's a good way to add a test for this?

Just cloned the repo, applied the fix, and added these tests to tests/unit/rc2.js line 18:

it('should expand a 127-bit key', function() {
  var key = UTIL.hexToBytes('88bca90e90875a7f0f79c384627bafb2');
  var expect = '71ab26462f0b9333609d4476e48ab72438c2194b70a47085d84b6af1dc72119023b94fe80aee2b6b45f27f923d9be1570da3ce8b16ad7f78db166ffbc28a836a4392cf0b748085dae4b69bdc2a4679cdfc09d84317016987e0c5b765c91dc612b1f44d7921b3e2c46447508bd2ac02e119e0f42a89c719675da320cf3e8958cd';
  ASSERT.equal(RC2.expandKey(key, 127).toHex(), expect);
});

it('should cut a 129-bit key', function() {
  var key = UTIL.hexToBytes('88bca90e90875a7f0f79c384627bafb216f80a6f85920584c42fceb0be255daf1e');
  var expect = '5b78d3a43dfff1f1';
  var cipher = RC2.createEncryptionCipher(key, 129);
  cipher.start(null);
  cipher.update(UTIL.createBuffer(UTIL.hexToBytes("0000000000000000")));
  cipher.finish();
  ASSERT.equal(cipher.output.toHex().slice(0, 16), expect);
});

The first test is just like the one above it, but for 127 bits. The second test is checking the last example in the spec. Feel free to adjust them if you like, or make other ones.