feat(protocol): fix sign malieability

packages/aztec.js/src/signer/index.js

@@ -5,6 +5,7 @@
  * @module signer
  */
 
+import BN from 'bn.js';
 import { constants, proofs } from '@aztec/dev-utils';
 import BN from 'bn.js';
 
@@ -314,5 +315,19 @@ signer.signTypedData = (domain, schema, message, privateKey) => {
         encodedTypedData,
     };
 };
+signer.makeReplaySignature = function makeReplaySignature(signatureToReplay) {
+    const [r, s, v] = signatureToReplay
+        .slice(2)
+        .match(/.{1,64}/g);
+    const secp256k1n = new BN("FFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFEBAAEDCE6AF48A03BBFD25E8CD0364141", 16);
+    const hex28 = (new BN(28)).toString(16);
+    const hex27 = (new BN(27)).toString(16);
+    const flippedS = secp256k1n
+        .sub(new BN(s, 16))
+        .toString(16);
+    const flippedV = (v === '1b') ? padRight(hex28.slice(-2), 64) : padRight(hex27.slice(-2), 64);
+    const reconstructedSig = r + flippedS + flippedV;
+    return `0x${reconstructedSig.slice(0, 130)}`
+}
 
 export default signer;

packages/extension/src/client/services/MetaMaskService/index.js

@@ -1,5 +1,6 @@
 import ethSigUtil from 'eth-sig-util';
 import * as ethUtil from 'ethereumjs-util';
+import { signer } from 'aztec.js';
 import Web3Service from '~/client/services/Web3Service';
 import registerExtension, { generateTypedData } from './registerExtension';
 import signNote from './signNote';
@@ -30,6 +31,7 @@ const handleAction = async (action, params) => {
         case 'metamask.register.extension': {
             const eip712Data = registerExtension(params);
             const method = 'eth_signTypedData_v4';
+
             const { result } = await Web3Service.sendAsync({
                 method,
                 params: [address, eip712Data],
@@ -49,33 +51,6 @@ const handleAction = async (action, params) => {
 
             break;
         }
-        case 'metamask.eip712.signNotes': {
-            const {
-                assetAddress,
-                noteHashes,
-                challenge,
-                sender,
-            } = params;
-            const signatures = (await Promise.all(noteHashes.map(async (noteHash) => {
-                const noteSchema = signNote({
-                    assetAddress,
-                    noteHash,
-                    challenge,
-                    sender,
-                });
-                const method = 'eth_signTypedData_v4';
-                return Web3Service.sendAsync({
-                    method,
-                    params: [address, noteSchema],
-                    from: address,
-                });
-            }))).map(({ result }) => result);
-
-            response = {
-                signatures,
-            };
-            break;
-        }
         case 'metamask.eip712.signProof': {
             const {
                 assetAddress,
@@ -97,7 +72,8 @@ const handleAction = async (action, params) => {
             });
 
             response = {
-                signature: result,
+                signature1: result,
+                signature2: signer.makeReplaySignature(result),
             };
 
             break;

packages/extension/src/config/contracts/development.js

@@ -1,4 +1,4 @@
-import AccountRegistry from '~contracts/Behaviour20200220.json';
+import AccountRegistry from '~contracts/Behaviour20200305.json';
 import AccountRegistryManager from '~contracts/IAccountRegistryManager.json';
 import ACE from '~contracts/ACE.json';
 import IERC20 from '~contracts/IERC20Mintable.json';

packages/extension/src/config/contracts/production.js

@@ -1,4 +1,4 @@
-import AccountRegistry from '~contracts/Behaviour20200220.json';
+import AccountRegistry from '~contracts/Behaviour20200305.json';
 import AccountRegistryManager from '~contracts/IAccountRegistryManager.json';
 import ACE from '~contracts/IACE.json';
 import IERC20 from '~contracts/IERC20Mintable.json';

packages/extension/src/ui/apis/asset/confidentialTransferFrom.js

@@ -10,7 +10,8 @@ const {
 export default async function confidentialTransferFrom({
     assetAddress,
     proof,
-    signature = '0x',
+    signature1 = '0x',
+    signature2 = '',
     spender,
     isGSNAvailable,
 }) {
@@ -23,7 +24,8 @@ export default async function confidentialTransferFrom({
             assetAddress,
             proofData,
             spender,
-            signature,
+            signature1,
+            signature2,
         ],
     };
     const response = isGSNAvailable

packages/protocol/contracts/AccountRegistry/epochs/20200305/Behaviour20200305.sol

@@ -0,0 +1,66 @@
+pragma solidity >=0.5.0 <0.6.0;
+
+import "../20200207/Behaviour20200207.sol";
+import "../../../interfaces/IERC20Permit.sol";
+
+/**
+ * @title Behaviour20200220 implementation
+ * @author AZTEC
+ * @dev This behaviour contract overloads the deposit() account registry method, with a deposit() 
+ * implementation that is compatible with the DAI permit() function.
+ * 
+ * Note the behaviour contract version naming convention is based on the date on which the contract
+ * was created, in the format: YYYYMMDD
+ * 
+ * Copyright 2020 Spilsbury Holdings Ltd 
+ *
+ * Licensed under the GNU Lesser General Public Licence, Version 3.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public License
+ * along with this program.  If not, see <https://www.gnu.org/licenses/>.
+ **/
+contract Behaviour20200220 is Behaviour20200207 {
+    /**
+    * @dev epoch number, used for version control in upgradeability. The naming convention is based on the
+    * date on which the contract was created, in the format: YYYYMMDD
+    */
+    uint256 public epoch = 20200305;
+
+    /**
+    * @dev Perform a confidential transfer, mediated by a smart contracrt
+    * @param _proofId - uint24 proofId
+    * @param _registryOwner - address of the note registry owner
+    * @param _proofData - data generated from proof construction, which is used to validate the proof
+    * @param _spender - address that will be spending the notes
+    * @param _proofSignature - EIP712 signature used to approve/revoke permission for the proof
+    * @param _proofSignature2 - EIP712 signature with be s bit flipped for replay protection of ZkDai 
+    * to be spent
+    */
+    function confidentialTransferFrom(
+        uint24 _proofId,
+        address _registryOwner,
+        bytes memory _proofData,
+        address _spender,
+        bytes memory _proofSignature,
+        bytes memory _proofSignature2
+    ) public {
+
+        bytes memory proofOutputs = ace.validateProof(_proofId, address(this), _proofData);
+
+        if(_proofSignature.length != 0) {
+            IZkAsset(_registryOwner).approveProof(_proofId, proofOutputs, _spender, true, _proofSignature);
+        }
+        if(_proofSignature2.length != 0) {
+            IZkAsset(_registryOwner).approveProof(_proofId, proofOutputs, _spender, true, _proofSignature2);
+        }
+
+        IZkAsset(_registryOwner).confidentialTransferFrom(_proofId, proofOutputs.get(0));
+    }
+}
+

packages/protocol/contracts/AccountRegistry/interfaces/IAccountRegistryBehaviour.sol

@@ -43,7 +43,8 @@ contract IAccountRegistryBehaviour {
         address _registryOwner,
         bytes calldata _proofData,
         address _spender,
-        bytes calldata _proofSignature   
+        bytes calldata _proofSignature,
+        bytes calldata _proofSignature2   
     ) external;
 
     function deposit(

packages/protocol/test/AccountRegistry/epochs/20200305/Behaviour20200305.spec.js

@@ -0,0 +1,166 @@
+/* globals contract, artifacts, expect */
+
+const {
+    JoinSplitProof,
+    note,
+    signer,
+} = require('aztec.js');
+const { constants, proofs } =require('@aztec/dev-utils');
+const { generateAccount } = require('@aztec/secp256k1');
+const truffleAssert = require('truffle-assertions');
+const { randomHex, toChecksumAddress } = require('web3-utils');
+
+const ACE = artifacts.require('./ACE');
+const AccountRegistryManager = artifacts.require('./AccountRegistry/AccountRegistryManager');
+const Behaviour20200220 = artifacts.require('./Behaviour20200106/epochs/20200106/Behaviour20200220');
+const Behaviour20200305 = artifacts.require('./Behaviour20200106/epochs/20200106/Behaviour20200220');
+const ERC20Mintable = artifacts.require('ERC20Mintable');
+const ZkAsset = artifacts.require('ZkAssetOwnable');
+const {
+    generateOutputNotes,
+    generateDepositProofInputs,
+    getOwnerPrivateKey,
+} = require('../../../helpers/AccountRegistry/epochs/20200106/Behaviour20200106');
+
+const standardAccount = require('../../../helpers/getOwnerAccount');
+const timetravel = require('../../../timeTravel');
+
+contract.only('Behaviour20200305', async (accounts) => {
+    const [userAddress, anotherUserAddress, senderAddress] = accounts;
+    let behaviour20200220;
+    let manager;
+    let proxyContract;
+    let erc20;
+    let ace;
+    let zkAsset;
+    let proxyAddress;
+    // Signature params
+    const opts = { from: accounts[0] };
+    const updatedGSNSignerAddress = '0x5323B6bbD3421983323b3f4f0B11c2D6D3bCE1d8';
+    const initialAmount = 100;
+
+    const chainID = 4;
+
+    beforeEach(async () => {
+        ace = await ACE.deployed();
+        erc20 = await ERC20Mintable.new({ from: senderAddress });
+        await erc20.mint(userAddress, initialAmount, { from: senderAddress });
+        zkAsset = await ZkAsset.new(ace.address, erc20.address, 1, {
+            from: accounts[2],
+        });
+
+        const behaviour20200220 = await Behaviour20200220.new();
+        const initialBehaviourAddress = behaviour20200220.address;
+        const initialGSNSignerAddress = randomHex(20);
+        manager = await AccountRegistryManager.new(initialBehaviourAddress, ace.address, initialGSNSignerAddress, opts);
+
+        // perform behaviour upgrade
+        behaviour20200305 = await Behaviour20200305.new();
+        await manager.upgradeAccountRegistry(behaviour20200305.address);
+        proxyAddress = await manager.proxyAddress();
+        proxyContract = await Behaviour20200305.at(proxyAddress);
+
+        // set the GSN signer
+        await proxyContract.setGSNSigner();
+    });
+
+    describe('Initialisation', async () => {
+        it('should have set the GSN signer address', async () => {
+            const updatedGSNSigner = await proxyContract.GSNSigner();
+            expect(updatedGSNSigner.toString()).to.equal(updatedGSNSignerAddress);
+        });
+
+        it('should deploy DAI contract', async () => {
+            expect(erc20.address).to.not.equal(undefined);
+        });
+    });
+
+    describe('Success states', async () => {
+        it('should call confidential approve twice if the a second signature is passed in', async () => {
+
+            const { inputNotes, outputNotes, publicValue, depositAmount } = await generateDepositProofInputs();
+            const publicOwner = proxyAddress;
+
+            const depositProof = new JoinSplitProof(inputNotes, outputNotes, proxyAddress, publicValue, publicOwner);
+            await erc20.approve(proxyAddress, depositAmount, { from: userAddress });
+
+            const depositProofData = depositProof.encodeABI(zkAsset.address);
+            const depositProofHash = depositProof.hash;
+            await proxyContract.deposit(zkAsset.address, userAddress, depositProofHash, depositProofData, depositAmount, {
+                from: userAddress,
+            });
+
+            // Use created output notes in a confidentialTransferFrom() call
+            const delegatedAddress = proxyAddress;
+            const transferInputNotes = outputNotes;
+            const transferOutputNotes = await generateOutputNotes([25, 25]);
+            const transferPublicValue = 0;
+            const transferPublicOwner = publicOwner;
+
+            const transferProof = new JoinSplitProof(
+                transferInputNotes,
+                transferOutputNotes,
+                delegatedAddress,
+                transferPublicValue,
+                transferPublicOwner,
+            );
+            const transferProofData = transferProof.encodeABI(zkAsset.address);
+            const delegatedAddressPrivateKey = getOwnerPrivateKey();
+
+            const proofSignature = signer.signApprovalForProof(
+                zkAsset.address,
+                transferProof.eth.outputs,
+                delegatedAddress,
+                true,
+                delegatedAddressPrivateKey,
+            );
+            const proofSignature2 = signer.makeReplaySignature(proofSignature);
+
+            const { receipt } = await proxyContract.confidentialTransferFrom(
+                proofs.JOIN_SPLIT_PROOF,
+                zkAsset.address,
+                transferProofData,
+                delegatedAddress,
+                proofSignature,
+                proofSignature2,
+            );
+            expect(receipt.status).to.equal(true);
+
+            await Promise.all(
+                transferOutputNotes.map(async (individualNote) => {
+                    const { status, noteOwner } = await ace.getNote(zkAsset.address, individualNote.noteHash);
+                    expect(status.toNumber()).to.equal(1);
+                    expect(noteOwner).to.equal(userAddress);
+                }),
+            );
+
+
+            await truffleAssert.reverts(
+                zkAsset.approveProof(proofs.JOIN_SPLIT_PROOF, transferProofData, delegatedAddress, true, proofSignature2),
+                'signature has already been used'
+            );
+
+
+        });
+
+    });
+    // it('should still be able to transfer if one signature is passed', async () => {
+    //     const holderAccount = generateAccount();
+    //     const { address: userAddress } = holderAccount;
+    //     const spender = randomHex(20);
+
+    //     const { receipt } = await proxyContract.methods['confidentialTransferFrom(uint24,address,bytes,address,bytes)'](
+    //         JOIN_SPLIT_PROOF,
+    //         zkAsset.address,
+    //         proofData,
+    //         userAddress,
+    //         signature,
+    //     );
+
+    //     console.log(receipt);
+
+
+    // });
+
+});
+