fix(crypto): improve handling of a share key being passed twice (#75)

Author: aduh95

packages/crypto/src/shamir.ts

@@ -189,17 +189,36 @@ export function split(
  */
 export function reconstruct(
   parts: BufferSource[],
-  neededParts: u8 = parts.length,
+  neededParts: u8 = undefined,
 ): Uint8Array {
-  if (parts.length < neededParts)
-    throw new Error("Not enough parts to reconstruct key");
   const bytes = parts[0].byteLength - 1;
   const result = new Uint8Array(bytes);
   const dataViews = parts.map(part =>
     ArrayBuffer.isView(part)
       ? new DataView(part.buffer, part.byteOffset, bytes + 1)
       : new DataView(part),
-  );
+  ).filter((shareView, i, shares) => {
+    const x = shareView.getUint8(bytes);
+    for (let j = 0; j < i; j++) {
+      const otherShareView = shares[j];
+      if (x !== otherShareView.getUint8(bytes)) continue;
+      for (let k = 0; k < bytes; k++) {
+        if (shareView.getUint8(k) !== otherShareView.getUint8(k)) {
+          throw new Error("There are conflicting key shares", { cause: [
+            parts[j],
+            parts[i],
+          ] });
+        }
+      }
+      return false;
+    }
+    return true;
+  });
+  if (neededParts == null) {
+    neededParts = dataViews.length;
+  } else if (dataViews.length < neededParts) {
+    throw new Error("Not enough parts to reconstruct key");
+  }
   for (let i = 0; i < bytes; i++) {
     result[i] = reconstructByte(
       Array.from({ length: neededParts }, (_, j) => {

packages/crypto/test/shamir.test.ts

@@ -13,6 +13,39 @@ it("should reconstruct a key", () => {
   );
 });
 
+describe("should handle a key part being passed twice", () => {
+  it("fail when there are not enough key parts", () => {
+    assert.notStrictEqual(
+      Buffer.from(shamir.reconstruct([
+        Buffer.from([0xef, 0x05, 0x70, 0x4a, 0xf2, 0xb2, 0xcd, 0x02]),
+        Buffer.from([0x62, 0x1e, 0x41, 0x63, 0xfa, 0x5e, 0x0b, 0x1c]),
+        Buffer.from([0xef, 0x05, 0x70, 0x4a, 0xf2, 0xb2, 0xcd, 0x02]),
+      ])).toString("hex"),
+      Buffer.from("caritat").toString("hex"),
+    );
+  });
+  it("throw if the are incompatible key parts", () => {
+    assert.throws(
+      () => shamir.reconstruct([
+        Buffer.from([0xef, 0x05, 0x70, 0x4a, 0xf2, 0xb2, 0xcd, 0x02]),
+        Buffer.from([0x62, 0x1e, 0x41, 0x63, 0xfa, 0x5e, 0x0b, 0x1c]),
+        Buffer.from([0xc4, 0xc8, 0x3c, 0x22, 0x53, 0x05, 0x62, 0x02]),
+      ]), /There are conflicting key shares/);
+  });
+  it("succeed when there are enough key parts", () => {
+    assert.strictEqual(
+      Buffer.from(shamir.reconstruct([
+        Buffer.from([0xef, 0x05, 0x70, 0x4a, 0xf2, 0xb2, 0xcd, 0x02]),
+        Buffer.from([0x62, 0x1e, 0x41, 0x63, 0xfa, 0x5e, 0x0b, 0x1c]),
+        Buffer.from([0xef, 0x05, 0x70, 0x4a, 0xf2, 0xb2, 0xcd, 0x02]),
+        Buffer.from([0xc4, 0xc8, 0x3c, 0x22, 0x53, 0x05, 0x62, 0x0a]),
+        Buffer.from([0xef, 0x05, 0x70, 0x4a, 0xf2, 0xb2, 0xcd, 0x02]),
+      ])).toString("hex"),
+      Buffer.from("caritat").toString("hex"),
+    );
+  });
+});
+
 const key = crypto.getRandomValues(new Uint8Array(256));
 
 describe("should reconstruct single byte with enough shareholders", () => {
@@ -95,9 +128,9 @@ it("should fail reconstruct key from not enough shareholders", () => {
 it("should fail reconstruct key with duplicate shareholders", () => {
   assert.throws(
     () => {
-      shamir.reconstruct([parts[1], parts[5], parts[1]]);
+      shamir.reconstruct([parts[1], parts[5], parts[1]], 3);
     },
-    { message: "Div/0" },
+    { message: "Not enough parts to reconstruct key" },
   );
 });