Reputation: 22722
I found this method in a .NET framework class, it uses the Bitwise &
operator to compare to byte arrays:
I think this functions is not optimal. For example we have two strings:
The third character is different when comparing the two strings. So when converting both into byte arrays, the third byte will be different and therefore the boolean variable flag will be false and will be false until the method finishes.
I would put behind this line:
flag = flag & a[i] == b[i];
if(flag==false)return false;
in order to prevent further loop execution. So why is this implementation as it is right now?
public static bool AreByteArraysEqual(byte[] a, byte[] b)
{
if (a == null || b == null || (int)a.Length != (int)b.Length)
{
return false;
}
bool flag = true;
for (int i = 0; i < (int)a.Length; i++)
{
flag = flag & a[i] == b[i];
}
return flag;
}
The class which holds the implementation resides in the System.Web.WebPages.dll, Version=3.0.0.0, Namespace System.Web.Helpers:
using System;
using System.Collections.Generic;
using System.IO;
using System.Security.Cryptography;
namespace System.Web.Helpers
{
internal static class CryptoUtil
{
public static bool AreByteArraysEqual(byte[] a, byte[] b)
{
if (a == null || b == null || (int)a.Length != (int)b.Length)
{
return false;
}
bool flag = true;
for (int i = 0; i < (int)a.Length; i++)
{
flag = flag & a[i] == b[i];
}
return flag;
}
public static byte[] ComputeSHA256(IList<string> parameters)
{
byte[] numArray;
using (MemoryStream memoryStream = new MemoryStream())
{
using (BinaryWriter binaryWriter = new BinaryWriter(memoryStream))
{
foreach (string parameter in parameters)
{
binaryWriter.Write(parameter);
}
binaryWriter.Flush();
using (SHA256Cng sHA256Cng = new SHA256Cng())
{
byte[] numArray1 = sHA256Cng.ComputeHash(memoryStream.GetBuffer(), 0, checked((int)memoryStream.Length));
numArray = numArray1;
}
}
}
return numArray;
}
}
}
Upvotes: 0
Views: 134
Reputation:
Your code looks like a decompiler's output. Don't look at a decompiler's output when the actual source code is available. It may have relevant comments, and does here:
// This method is specially written to take the same amount of time
// regardless of where 'a' and 'b' differ. Please do not optimize it.
public static bool AreByteArraysEqual(byte[] a, byte[] b)
...
The improvement you've made would mean that the time needed to evaluate AreByteArraysEqual
for two byte arrays of equal length depends on how many of the initial bytes match. Normally that's not a problem. For this method, it is, because it would allow the caller to obtain data (by continually trying until the first byte is right, then continually trying until the second byte is right, and so on) that's meant to remain secret.
Upvotes: 7