fix(signatures): reject ecrecover bypass

src/Util/Signatures.sol

@@ -1,9 +1,9 @@
 // SPDX-License-Identifier: MIT
 pragma solidity ^0.8.19;
 
-library Signatures {
-    error InvalidSignatureLength();
+import "@openzeppelin/contracts/utils/cryptography/ECDSA.sol";
 
+library Signatures {
     /** Hash of the message to sign */
     function getPostageMessageHash(
         bytes32 _chunkAddr,
@@ -25,7 +25,7 @@ library Signatures {
         bytes32 messageHash = getPostageMessageHash(_chunkAddr, _postageId, _index, _timeStamp);
         bytes32 ethMessageHash = getEthSignedMessageHash(messageHash);
 
-        return recoverSigner(ethMessageHash, _signature) == _signer;
+        return verifySignature(ethMessageHash, _signature, _signer);
     }
 
     function getEthSignedMessageHash(bytes32 _messageHash) internal pure returns (bytes32) {
@@ -36,38 +36,13 @@ library Signatures {
         return keccak256(abi.encodePacked("\x19Ethereum Signed Message:\n32", _messageHash));
     }
 
-    function recoverSigner(
+    function verifySignature(
         bytes32 _ethSignedMessageHash, // it has to be prefixed message: https://ethereum.stackexchange.com/questions/19582/does-ecrecover-in-solidity-expects-the-x19ethereum-signed-message-n-prefix/21037
-        bytes memory _signature
-    ) internal pure returns (address) {
-        (bytes32 r, bytes32 s, uint8 v) = splitSignature(_signature);
-
-        return ecrecover(_ethSignedMessageHash, v, r, s);
-    }
-
-    function splitSignature(bytes memory sig) internal pure returns (bytes32 r_, bytes32 s_, uint8 v_) {
-        if (sig.length != 65) {
-            revert InvalidSignatureLength();
-        }
-
-        assembly {
-            /*
-            verbose explanation: https://ethereum.stackexchange.com/questions/135591/split-signature-function-in-solidity-by-example-docs
-            First 32 bytes stores the length of the signature
-            add(sig, 32) = pointer of sig + 32
-            effectively, skips first 32 bytes of signature
-            mload(p) loads next 32 bytes starting at the memory address p into memory
-            */
-
-            // first 32 bytes, after the length prefix
-            r_ := mload(add(sig, 32))
-            // second 32 bytes
-            s_ := mload(add(sig, 64))
-            // final byte (first byte of the next 32 bytes)
-            v_ := byte(0, mload(add(sig, 96)))
-        }
-
-        // implicitly return (r, s, v)
+        bytes memory _signature,
+        address _signer
+    ) internal pure returns (bool) {
+        (address recovered, ECDSA.RecoverError error) = ECDSA.tryRecover(_ethSignedMessageHash, _signature);
+        return error == ECDSA.RecoverError.NoError && recovered == _signer;
     }
 
     function getSocMessageHash(bytes32 _identifier, bytes32 _chunkAddr) internal pure returns (bytes32) {
@@ -83,6 +58,6 @@ library Signatures {
         bytes32 messageHash = getSocMessageHash(_identifier, _chunkAddr);
         bytes32 ethMessageHash = getEthSignedMessageHash(messageHash);
 
-        return recoverSigner(ethMessageHash, _signature) == _signer;
+        return verifySignature(ethMessageHash, _signature, _signer);
     }
 }

src/test/SignaturesHarness.sol

@@ -0,0 +1,27 @@
+// SPDX-License-Identifier: MIT
+pragma solidity ^0.8.19;
+
+import "../Util/Signatures.sol";
+
+/// @dev Thin wrapper for unit-testing the Signatures library.
+contract SignaturesHarness {
+    function socVerify(
+        address signer,
+        bytes memory signature,
+        bytes32 identifier,
+        bytes32 chunkAddr
+    ) external pure returns (bool) {
+        return Signatures.socVerify(signer, signature, identifier, chunkAddr);
+    }
+
+    function postageVerify(
+        address signer,
+        bytes memory signature,
+        bytes32 chunkAddr,
+        bytes32 postageId,
+        uint64 index,
+        uint64 timeStamp
+    ) external pure returns (bool) {
+        return Signatures.postageVerify(signer, signature, chunkAddr, postageId, index, timeStamp);
+    }
+}

test/Redistribution.test.ts

@@ -1173,6 +1173,18 @@ describe('Redistribution', function () {
             ).to.be.revertedWith(errors.claim.socVerificationFailed);
           });
 
+          it('rejects SOC address(0) signer bypass at claim', async function () {
+            const { proofParams } = await generatedSampling(true);
+            const socProof = proofParams.proof1.socProof![0];
+
+            socProof.signer = ethers.constants.AddressZero;
+            socProof.signature = '0x' + '00'.repeat(65);
+
+            await expect(
+              r_node_5.claim(proofParams.proof1, proofParams.proof2, proofParams.proofLast)
+            ).to.be.revertedWith(errors.claim.socVerificationFailed);
+          });
+
           it('SOC attachment does not match with witness', async function () {
             const { proofParams } = await generatedSampling(true);
 

test/Signatures.test.ts

@@ -0,0 +1,91 @@
+import { expect } from './util/chai';
+import { ethers } from 'hardhat';
+import { hexlify } from 'ethers/lib/utils';
+import { randomBytes } from 'crypto';
+
+describe('Signatures', () => {
+  let harness: Awaited<ReturnType<typeof deployHarness>>;
+
+  async function deployHarness() {
+    const factory = await ethers.getContractFactory('SignaturesHarness');
+    return factory.deploy();
+  }
+
+  before(async () => {
+    harness = await deployHarness();
+  });
+
+  it('rejects SOC verification bypass via address(0) and malformed signature', async () => {
+    const identifier = randomBytes(32);
+    const chunkAddr = randomBytes(32);
+    const malformedSignature = '0x' + '00'.repeat(65);
+
+    const verified = await harness.socVerify(
+      ethers.constants.AddressZero,
+      malformedSignature,
+      hexlify(identifier),
+      hexlify(chunkAddr)
+    );
+
+    expect(verified).to.be.false;
+  });
+
+  it('rejects SOC verification with invalid signature length', async () => {
+    const identifier = randomBytes(32);
+    const chunkAddr = randomBytes(32);
+    const shortSignature = '0x' + '00'.repeat(64);
+
+    const verified = await harness.socVerify(
+      ethers.Wallet.createRandom().address,
+      shortSignature,
+      hexlify(identifier),
+      hexlify(chunkAddr)
+    );
+
+    expect(verified).to.be.false;
+  });
+
+  it('rejects SOC verification with high-s malleable signature', async () => {
+    const wallet = ethers.Wallet.createRandom();
+    const identifier = randomBytes(32);
+    const chunkAddr = randomBytes(32);
+    const messageHash = ethers.utils.solidityKeccak256(['bytes32', 'bytes32'], [identifier, chunkAddr]);
+    const signature = await wallet.signMessage(ethers.utils.arrayify(messageHash));
+
+    const { r, s, v } = ethers.utils.splitSignature(signature);
+    const malleableS = ethers.BigNumber.from(
+      '0xFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFEBAAEDCE6AF48A03BBFD25E8CD0364141'
+    ).sub(ethers.BigNumber.from(s));
+    const malleableSignature = ethers.utils.hexConcat([
+      r,
+      ethers.utils.hexZeroPad(malleableS.toHexString(), 32),
+      ethers.utils.hexlify(v === 27 ? 28 : 27),
+    ]);
+
+    const verified = await harness.socVerify(
+      wallet.address,
+      malleableSignature,
+      hexlify(identifier),
+      hexlify(chunkAddr)
+    );
+
+    expect(verified).to.be.false;
+  });
+
+  it('accepts a valid SOC signature', async () => {
+    const wallet = ethers.Wallet.createRandom();
+    const identifier = randomBytes(32);
+    const chunkAddr = randomBytes(32);
+    const messageHash = ethers.utils.solidityKeccak256(['bytes32', 'bytes32'], [identifier, chunkAddr]);
+    const signature = await wallet.signMessage(ethers.utils.arrayify(messageHash));
+
+    const verified = await harness.socVerify(
+      wallet.address,
+      signature,
+      hexlify(identifier),
+      hexlify(chunkAddr)
+    );
+
+    expect(verified).to.be.true;
+  });
+});