kitsook/ChaCha20

Probable bug

Closed this issue · 4 comments

This line reads

if (this.matrix[12] <= 0) {

but should most probably be

if (this.matrix[12] == 0) {

as unsigned overflow occurs when transitioning from 0xFFFFFFFF to 0x00000000.

Hi @Maaartinus, thx for your comment. Before Java 8, all integers are signed. Even so, handling of unsigned integer requires the use of methods defined in Integer class.

As you may have noticed, that chunck of code probably need to be revised to handle 32-bit vs 64-bit and the signed vs unsigned, which is pending #4

Hi @Maaartinus, thx for your comment. Before Java 8, all integers are signed. Even so, handling of unsigned integer requires the use of methods defined in Integer class.

Sure, but that doesn't matter. When doing modular addition, subtraction or multiplication, signedness just doesn't matter. For division, parsing, stringifying, etc., it does.

For multiple precision addition, it does not. Having unsigned int won't help you at all. Imagine a two-byte counter starting with (hexadecimal) 00_00. You increment it to 00_01, 00_02, ..., 00_7F and then to 00_80, which looks like negative, but this doesn't matter. We continue to 00_81, ..., 00_FF and then to 01_00. The carry into the higher byte occurs if and only if the lower byte becomes zero. No magic and no sign involved.

Note that your condition increments the higher part multiple times in row, which is surely wrong.

--

Sure, after switching to the new version using a 32 bit counter, this problem goes away.

I see your point. will create some test cases with enough data to test that out. == 0 seems logical

@kitsook any news on this? It is now more than one year and you still didn't change anything.^^