BitArray.xor() should check length of both arrays
Closed this issue · 3 comments
Anlo2846 commented
Annotated code from Source/lib/common/BitArray.cs, lines 346-362:
/// <summary>
/// XOR operation
/// </summary>
/// <param name="other"></param>
public void xor(BitArray other)
{
if (size != other.size)
{
throw new ArgumentException("Sizes don't match");
}
for (int i = 0; i < bits.Length; i++) // Anlo2846: Checking only one array length, other.bits might be shorter
{
// The last int could be incomplete (i.e. not have 32 bits in
// it) but there is no problem since 0 XOR 0 == 0.
bits[i] ^= other.bits[i];
}
}
Code to reproduce bug:
using ZXing.Common;
BitArray a = new BitArray(128); // a.bits.Length == 4
a.appendBit(true); // a.bits.Length == 6
BitArray b = new BitArray(129); // b.bits.Length == 5
b[128] = true;
a.xor(b); // System.IndexOutOfRangeException: 'Index was outside the bounds of the array.'
Suggested fix in Source/lib/common/BitArray.cs, line 356:
for (int i = 0; i < bits.Length && i < other.bits.Length; i++)
micjahn commented
A really interesting error, thanks for reporting it.
What do you think about the following fix:
var numberOfBits = bits.Length;
if (other.bits.Length < numberOfBits)
numberOfBits = other.bits.Length;
for (int i = 0; i < numberOfBits; i++)
{
// The last int could be incomplete (i.e. not have 32 bits in
// it) but there is no problem since 0 XOR 0 == 0.
bits[i] ^= other.bits[i];
}
Anlo2846 commented
Yeah, that will work. But I would name the variable numberOfInts (or similar) instead since it is not really the number of bits.
micjahn commented
You are right. I named it that way because of the name of the array. But numberOfBits should be clearer.