Fix double ownership creation malicious user bug

src/assets/assets.cpp

@@ -665,6 +665,16 @@ bool CTransaction::VerifyNewAsset() const
     if (vout.size() < 3)
         return false;
 
+    // Loop through all of the vouts and make sure only the expected asset creations are taking place
+    int nTransfers = 0;
+    int nOwners = 0;
+    int nIssues = 0;
+    int nReissues = 0;
+    GetTxOutAssetTypes(vout, nIssues, nReissues, nTransfers, nOwners);
+
+    if (nOwners != 1 || nIssues != 1 || nReissues > 0)
+        return false;
+
     // Check for the assets data CTxOut. This will always be the last output in the transaction
     if (!CheckIssueDataTx(vout[vout.size() - 1]))
         return false;
@@ -785,6 +795,18 @@ bool CTransaction::VerifyNewUniqueAsset(CCoinsViewCache& view) const
     if (!fFoundCorrectInput)
         return false;
 
+    // Loop through all of the vouts and make sure only the expected asset creations are taking place
+    int nTransfers = 0;
+    int nOwners = 0;
+    int nIssues = 0;
+    int nReissues = 0;
+    GetTxOutAssetTypes(vout, nIssues, nReissues, nTransfers, nOwners);
+
+    if (nOwners > 0 || nReissues > 0 || nIssues != assetOutpointCount) {
+        return false;
+    }
+
+
     return true;
 }
 
@@ -855,6 +877,17 @@ bool CTransaction::VerifyReissueAsset(CCoinsViewCache& view) const
         if (CheckReissueBurnTx(out))
             return true;
 
+
+    // Loop through all of the vouts and make sure only the expected asset creations are taking place
+    int nTransfers = 0;
+    int nOwners = 0;
+    int nIssues = 0;
+    int nReissues = 0;
+    GetTxOutAssetTypes(vout, nIssues, nReissues, nTransfers, nOwners);
+
+    if (nOwners > 0 || nReissues != 1 || nIssues > 0)
+        return false;
+
     return false;
 }
 
@@ -2802,4 +2835,22 @@ bool CheckEncodedIPFS(const std::string& hash, std::string& strError)
     }
 
     return true;
+}
+
+void GetTxOutAssetTypes(const std::vector<CTxOut>& vout, int& issues, int& reissues, int& transfers, int& owners)
+{
+    for (auto out: vout) {
+        int type;
+        bool fIsOwner;
+        if (out.scriptPubKey.IsAssetScript(type, fIsOwner)) {
+            if (type == TX_NEW_ASSET && !fIsOwner)
+                issues++;
+            else if (type == TX_NEW_ASSET && fIsOwner)
+                owners++;
+            else if (type == TX_TRANSFER_ASSET)
+                transfers++;
+            else if (type == TX_REISSUE_ASSET)
+                reissues++;
+        }
+    }
 }

src/assets/assets.h

@@ -315,6 +315,8 @@ CAmount GetBurnAmount(const int nType);
 std::string GetBurnAddress(const AssetType type);
 std::string GetBurnAddress(const int nType);
 
+void GetTxOutAssetTypes(const std::vector<CTxOut>& vout, int& issues, int& reissues, int& transfers, int& owners);
+
 bool IsAssetNameValid(const std::string& name);
 bool IsAssetNameValid(const std::string& name, AssetType& assetType);
 bool IsAssetNameValid(const std::string& name, AssetType& assetType, std::string& error);

src/rpc/rawtransaction.cpp

@@ -591,11 +591,18 @@ UniValue createrawtransaction(const JSONRPCRequest& request)
                     // Construct the asset transaction
                     asset.ConstructTransaction(scriptPubKey);
 
-                    asset.ConstructOwnerTransaction(ownerPubKey);
-
-                    // Push the scriptPubKey into the vouts.
-                    CTxOut ownerOut(0, ownerPubKey);
-                    rawTx.vout.push_back(ownerOut);
+                    AssetType type;
+                    if (IsAssetNameValid(asset.strName, type)) {
+                        if (type != AssetType::UNIQUE) {
+                            asset.ConstructOwnerTransaction(ownerPubKey);
+
+                            // Push the scriptPubKey into the vouts.
+                            CTxOut ownerOut(0, ownerPubKey);
+                            rawTx.vout.push_back(ownerOut);
+                        }
+                    } else {
+                        throw JSONRPCError(RPC_INVALID_PARAMETER, ("Invalid parameter, invalid asset name"));
+                    }
 
                     // Push the scriptPubKey into the vouts.
                     CTxOut out(0, scriptPubKey);

test/functional/feature_rawassettransactions.py

@@ -184,6 +184,20 @@ def issue_tampering_test(self):
         assert_raises_rpc_error(-26, "bad-txns-bad-asset-transaction",
                                 n0.sendrawtransaction, tx_bad_issue_signed)
 
+        ########################################
+        # try tampering to issue an asset with duplicate owner outputs
+        tx = CTransaction()
+        f = BytesIO(hex_str_to_bytes(tx_issue_hex))
+        tx.deserialize(f)
+        rvno = '72766e6f' #rvno
+        # find the owner output from vout and insert a duplicate back in
+        owner_vout = list(filter(lambda out : rvno in bytes_to_hex_str(out.scriptPubKey), tx.vout))[0]
+        tx.vout.insert(-1, owner_vout)
+        tx_bad_issue = bytes_to_hex_str(tx.serialize())
+        tx_bad_issue_signed = n0.signrawtransaction(tx_bad_issue)['hex']
+        assert_raises_rpc_error(-26, "bad-txns-verifying-issue-asset",
+                                n0.sendrawtransaction, tx_bad_issue_signed)
+
         ########################################
         # try tampering to issue an owner token with no asset
         tx = CTransaction()
@@ -508,7 +522,7 @@ def unique_assets_test(self):
         root = "RINGU"
         owner = f"{root}!"
         n0.issue(root)
-        n0.generate(1)
+        n0.generate(10)
 
         asset_tags = ["myprecious1", "bind3", "gold7", "men9"]
         ipfs_hashes = ["QmWWQSuPMS6aXCbZKpEjPHPUZN2NjB3YrhJTHsV4X3vb2t"] * len(asset_tags)