PointyCastle / pointycastle

Moved into the Bouncy Castle project: https://github.com/bcgit/pc-dart
MIT License
271 stars 75 forks source link

Wrong result when using Uint8List that's a partial view of a ByteBuffer #110

Closed stevenroose closed 7 years ago

stevenroose commented 7 years ago

(@izaera I find it strange that you didn't think if this :p)

Due to Dart's lack of a native type for byte arrays, Pointy Castle, and most Dart developers, use Uint8List as a replacement. However, Uint8List is only a view on a ByteBuffer.

Consider the example:

Uint8List bytes1 = new Uint8List.fromList([0,1,2,3,4,5,6,7,8,9]);
Uint8List bytes2 = new Uint8List.view(bytes1.buffer, 1);
print(bytes2); // [1,2,3,4,5,6,7,8,9]
print(bytes1.buffer == bytes2.buffer); // true
print(bytes2.offsetInBytes); // 1

Now look at this method: https://github.com/PointyCastle/pointycastle/blob/master/lib/src/ufixnum.dart#L193 The inp field is directly passed along from a Digest.update call. So when we pass bytes2 in there, the ByteData view will use the same ByteBuffer as bytes1 has.

What that line should be is

inp = new ByteData.view(inp.buffer, inp.offsetInBytes, inp.length);

But then again. This is just a while trickery around the bad design of Dart's types.

stevenroose commented 7 years ago

Will fix, write extra tests for ufixnum and look for other cases were this happens.

stevenroose commented 7 years ago

This is bad, a lot of my tests in Dartcoin failed once I pulled this update, even though the Pointy Castle ones didn't. Let me see this through. I'm gonna push a roll back release for now so I don't break people's code.

stevenroose commented 7 years ago

Turned out to be a typo. Added extra tests and pushed the fix.