CryptoHelper::compareArray - leaks info about arrays

[GoogleCodeExporter]

Oops... three cases where the comment does not hold. At minimum, it might be 
appropriate to spin based on case 3's min/avg/max length.

if ( b1 == b2 ) {
  return true;
}
if ( b1 == null || b2 == null ) {
  return (b1 == b2);
}
if ( b1.length != b2.length ) {
  return false;
}

int result = 0;
// Make sure to go through ALL the bytes. We use the fact that if
// you XOR any bit stream with itself the result will be all 0 bits,
// which in turn yields 0 for the result.
for(int i = 0; i < b1.length; i++) {
  // XOR the 2 current bytes and then OR with the outstanding result.
  result |= (b1[i] ^ b2[i]);
}
return (result == 0) ? true : false;

Original issue reported on code.google.com by noloa...@gmail.com on 6 Aug 2011 at 8:53

[GoogleCodeExporter]

I implemented something similar myself. It seems to me the object equality test 
is ok simply because it's comparing actual object references and shouldn't be 
encountered in a real data compared with expected data scenario.

The null case is similar, a normal real vs expected case should result in the 
"real" value being at least a zero length string.

I agree about the third case. That can leak the length of the expected value 
very easily. My fix for that was:

int result = b1.length == b2.length ? 0 : 1;
int pinned = (Math.max(b1.length,  b2.length) / 128 + 1) * 128;

for (int i = 0; i < pinned; i++)
{
  result |= (b1[i % b1.length] ^ b2[i % b2.length]);
}

In essence, it loops to the block that is greater in size of both arrays. I 
used 128 here, but that could be changed. Since I initially check the length, 
they have already been identified as not being equals so the loop is just going 
through the motions. The modulus part allows it to wrap around and keep going 
till it hits the pinned value.

Original comment by b...@copperleaf.org on 18 Jan 2013 at 5:26