prevent applicant from hijacking reserved addresses and send tribute …

…from failed proposals to proposer not applicant

contracts/Moloch.sol

@@ -182,6 +182,7 @@ contract Moloch is ReentrancyGuard {
         require(tokenWhitelist[tributeToken], "tributeToken is not whitelisted");
         require(tokenWhitelist[paymentToken], "payment is not whitelisted");
         require(applicant != address(0), "applicant cannot be 0");
+        require(applicant != GUILD && applicant != ESCROW && applicant != TOTAL, "applicant address cannot be reserved");
         require(members[applicant].jailed == 0, "proposal applicant must not be jailed");
 
         if (tributeOffered > 0 && userTokenBalances[GUILD][tributeToken] == 0) {
@@ -411,8 +412,8 @@ contract Moloch is ReentrancyGuard {
 
         // PROPOSAL FAILED
         } else {
-            // return all tokens to the applicant
-            unsafeInternalTransfer(ESCROW, proposal.applicant, proposal.tributeToken, proposal.tributeOffered);
+            // return all tokens to the proposer (not the applicant, because funds come from proposer)
+            unsafeInternalTransfer(ESCROW, proposal.proposer, proposal.tributeToken, proposal.tributeOffered);
         }
 
         _returnDeposit(proposal.sponsor);

test/molochV2.test.js

@@ -43,6 +43,7 @@ const revertMessages = {
   submitProposalTributeTokenIsNotWhitelisted: 'tributeToken is not whitelisted',
   submitProposalPaymetTokenIsNotWhitelisted: 'payment is not whitelisted',
   submitProposalApplicantCannotBe0: 'revert applicant cannot be 0',
+  submitProposalApplicantCannotBeReserved: 'applicant address cannot be reserved',
   submitProposalApplicantIsJailed: 'proposal applicant must not be jailed',
   submitWhitelistProposalMustProvideTokenAddress: 'must provide token address',
   submitWhitelistProposalAlreadyHaveWhitelistedToken: 'cannot already have whitelisted the token',
@@ -91,6 +92,7 @@ const SolRevert = 'VM Exception while processing transaction: revert'
 const zeroAddress = '0x0000000000000000000000000000000000000000'
 const GUILD  = '0x000000000000000000000000000000000000dead'
 const ESCROW = '0x000000000000000000000000000000000000beef'
+const TOTAL = '0x000000000000000000000000000000000000babe'
 const MAX_TOKEN_WHITELIST_COUNT = new BN('10') // TODO: actual number to be determined
 
 const _1 = new BN('1')
@@ -624,6 +626,44 @@ contract('Moloch', ([creator, summoner, applicant1, applicant2, processor, deleg
       ).should.be.rejectedWith(revertMessages.submitProposalApplicantCannotBe0)
     })
 
+    it('require fail - applicant address can not be reserved', async () => {
+      await moloch.submitProposal(
+        GUILD,
+        proposal1.sharesRequested,
+        proposal1.lootRequested,
+        proposal1.tributeOffered,
+        proposal1.tributeToken,
+        proposal1.paymentRequested,
+        proposal1.paymentToken,
+        proposal1.details,
+        { from: proposal1.applicant }
+      ).should.be.rejectedWith(revertMessages.submitProposalApplicantCannotBeReserved)
+
+      await moloch.submitProposal(
+        ESCROW,
+        proposal1.sharesRequested,
+        proposal1.lootRequested,
+        proposal1.tributeOffered,
+        proposal1.tributeToken,
+        proposal1.paymentRequested,
+        proposal1.paymentToken,
+        proposal1.details,
+        { from: proposal1.applicant }
+      ).should.be.rejectedWith(revertMessages.submitProposalApplicantCannotBeReserved)
+
+      await moloch.submitProposal(
+        TOTAL,
+        proposal1.sharesRequested,
+        proposal1.lootRequested,
+        proposal1.tributeOffered,
+        proposal1.tributeToken,
+        proposal1.paymentRequested,
+        proposal1.paymentToken,
+        proposal1.details,
+        { from: proposal1.applicant }
+      ).should.be.rejectedWith(revertMessages.submitProposalApplicantCannotBeReserved)    
+    })
+
     it('failure - too many shares requested', async () => {
       await moloch.submitProposal(
         proposal1.applicant,
@@ -2002,15 +2042,16 @@ contract('Moloch', ([creator, summoner, applicant1, applicant2, processor, deleg
       })
     })
 
-    it('happy path - fail - no wins', async () => {
+    it('happy path - fail - no wins (proposer gets funds back)', async () => {
+      proposer = proposal2.applicant // need to test that funds go back to proposer, not applicant
+
       await fundAndApproveToMoloch({
-        to: proposal1.applicant,
+        to: proposer, // approve funds from proposer, not applicant
         from: creator,
         value: proposal1.tributeOffered
       })
 
       // submit
-      proposer = proposal1.applicant
       applicant = proposal1.applicant
       await moloch.submitProposal(
         applicant,
@@ -2101,7 +2142,7 @@ contract('Moloch', ([creator, summoner, applicant1, applicant2, processor, deleg
         userBalances: {
           [GUILD]: 0,
           [ESCROW]: 0,
-          [proposal1.applicant]: proposal1.tributeOffered,
+          [proposer]: proposal1.tributeOffered,
           [summoner]: deploymentConfig.PROPOSAL_DEPOSIT - deploymentConfig.PROCESSING_REWARD,
           [processor]: deploymentConfig.PROCESSING_REWARD
         }