From 5f35aafc29ccee41e1f293aa3f453e3b7c9ce4fe Mon Sep 17 00:00:00 2001
From: Ivan Socolsky <jungans@gmail.com>
Date: Thu, 28 Jan 2016 11:01:03 -0300
Subject: [PATCH] fix proposal signature validation in publishTx

---
 lib/server.js              | 22 +++++++++-------------
 test/integration/server.js | 36 +++++++++++++++++-------------------
 2 files changed, 26 insertions(+), 32 deletions(-)

diff --git a/lib/server.js b/lib/server.js
index 9b731c1..dc7f857 100644
--- a/lib/server.js
+++ b/lib/server.js
@@ -650,8 +650,8 @@ WalletService.prototype.joinWallet = function(opts, cb) {
       }
 
       if (_.find(wallet.copayers, {
-          xPubKey: opts.xPubKey
-        })) return cb(Errors.COPAYER_IN_WALLET);
+        xPubKey: opts.xPubKey
+      })) return cb(Errors.COPAYER_IN_WALLET);
 
       if (wallet.copayers.length == wallet.n) return cb(Errors.WALLET_FULL);
 
@@ -744,8 +744,8 @@ WalletService.prototype._canCreateAddress = function(ignoreMaxGap, cb) {
       isChange: true
     }), Defaults.MAX_MAIN_ADDRESS_GAP);
     if (latestAddresses.length < Defaults.MAX_MAIN_ADDRESS_GAP || _.any(latestAddresses, {
-        hasActivity: true
-      })) return cb(null, true);
+      hasActivity: true
+    })) return cb(null, true);
 
     var bc = self._getBlockchainExplorer(latestAddresses[0].network);
     var activityFound = false;
@@ -1618,8 +1618,6 @@ WalletService.prototype._verifyRequestPubKey = function(requestPubKey, signature
  * @param {Object} opts
  * @param {string} opts.txProposalId - The tx id.
  * @param {string} opts.proposalSignature - S(raw tx). Used by other copayers to verify the proposal.
- * @param {string} opts.proposalSignaturePubKey - (Optional) An alternative public key used to verify the proposal signature.
- * @param {string} opts.proposalSignaturePubKeySig - (Optional) A signature used to validate the opts.proposalSignaturePubKey.
  */
 WalletService.prototype.publishTx = function(opts, cb) {
   var self = this;
@@ -1648,13 +1646,11 @@ WalletService.prototype.publishTx = function(opts, cb) {
           return cb(new ClientError('Invalid proposal signature'));
         }
 
-        // Verify signingKey
-        if (opts.proposalSignaturePubKey) {
-          if (opts.proposalSignaturePubKey != signingKey ||
-            !self._verifyRequestPubKey(opts.proposalSignaturePubKey, opts.proposalSignaturePubKeySig, copayer.xPubKey)
-          ) {
-            return cb(new ClientError('Invalid proposal signing key'));
-          }
+        // Save signature info for other copayers to check
+        txp.proposalSignature = opts.proposalSignature;
+        if (signingKey.selfSigned) {
+          txp.proposalSignaturePubKey = signingKey.key;
+          txp.proposalSignaturePubKeySig = signingKey.signature;
         }
 
         // Verify UTXOs are still available
diff --git a/test/integration/server.js b/test/integration/server.js
index b245558..c2bbd67 100644
--- a/test/integration/server.js
+++ b/test/integration/server.js
@@ -2739,6 +2739,7 @@ describe('Wallet service', function() {
               server.getPendingTxs({}, function(err, txs) {
                 should.not.exist(err);
                 txs.length.should.equal(1);
+                should.exist(txs[0].proposalSignature);
                 done();
               });
             });
@@ -2795,21 +2796,14 @@ describe('Wallet service', function() {
             should.not.exist(err);
             should.exist(txp);
 
-            var raw = txp.getRawTx();
-            var proposalSignature = helpers.signMessage(raw, TestData.copayers[0].privKey_1H_0);
-            var pubKey = new Bitcore.PrivateKey(TestData.copayers[0].privKey_1H_0).toPublicKey().toString();
-            var pubKeySig = helpers.signMessage(pubKey, TestData.copayers[1].privKey_1H_0);
-
             var publishOpts = {
               txProposalId: txp.id,
-              proposalSignature: proposalSignature,
-              proposalSignaturePubKey: pubKey,
-              proposalSignaturePubKeySig: pubKeySig,
+              proposalSignature: helpers.signMessage(txp.getRawTx(), TestData.copayers[1].privKey_1H_0),
             }
 
             server.publishTx(publishOpts, function(err) {
               should.exist(err);
-              err.message.should.contain('Invalid proposal signing key');
+              err.message.should.contain('Invalid proposal signature');
               done();
             });
           });
@@ -2818,18 +2812,17 @@ describe('Wallet service', function() {
 
       it('should accept a tx proposal signed with a custom key', function(done) {
         var reqPrivKey = new Bitcore.PrivateKey();
-        var reqPubKey = reqPrivKey.toPublicKey();
+        var reqPubKey = reqPrivKey.toPublicKey().toString();
 
         var xPrivKey = TestData.copayers[0].xPrivKey_44H_0H_0H;
-        var sig = helpers.signRequestPubKey(reqPubKey, xPrivKey);
 
-        var opts = {
+        var accessOpts = {
           copayerId: TestData.copayers[0].id44,
           requestPubKey: reqPubKey,
-          signature: sig,
+          signature: helpers.signRequestPubKey(reqPubKey, xPrivKey),
         };
 
-        server.addAccess(opts, function(err) {
+        server.addAccess(accessOpts, function(err) {
           should.not.exist(err);
 
           helpers.stubUtxos(server, wallet, [1, 2], function() {
@@ -2846,14 +2839,19 @@ describe('Wallet service', function() {
               var publishOpts = {
                 txProposalId: txp.id,
                 proposalSignature: helpers.signMessage(txp.getRawTx(), reqPrivKey),
-                proposalSignaturePubKey: reqPubKey,
-                proposalSignaturePubKeySig: sig,
               }
 
               server.publishTx(publishOpts, function(err) {
-                should.exist(err);
-                err.message.should.contain('Invalid proposal signing key');
-                done();
+                should.not.exist(err);
+                server.getTx({
+                  txProposalId: txp.id
+                }, function(err, x) {
+                  should.not.exist(err);
+                  x.proposalSignature.should.equal(publishOpts.proposalSignature);
+                  x.proposalSignaturePubKey.should.equal(accessOpts.requestPubKey);
+                  x.proposalSignaturePubKeySig.should.equal(accessOpts.signature);
+                  done();
+                });
               });
             });
           });