Merge #144: [Wallet] Reduce usage of atoi to comply with CWE-190

c7e3d31 Reduce usage of atoi to comply with CWE-190: Integer Overflow or Wraparound https://cwe.mitre.org/data/definitions/190.html (presstab) Tree-SHA512: 67ddb70f63593fa085e094f7d908d9f37fb54a614b29d959f0cac5e73c7d8bcb87da1f4665f0c3b915bcea089d8a72bdfea8c8426faa826ec63d1a440686aa90
PIVX-Project · Apr 26, 2017 · e4e68bc · e4e68bc
2 parents 238977b + c7e3d31
commit e4e68bc
Show file tree

Hide file tree

Showing 7 changed files with 82 additions and 13 deletions.
diff --git a/src/activemasternode.cpp b/src/activemasternode.cpp
@@ -372,7 +372,14 @@ bool CActiveMasternode::GetMasterNodeVin(CTxIn& vin, CPubKey& pubkey, CKey& secr
     if (!strTxHash.empty()) {
         // Let's find it
         uint256 txHash(strTxHash);
-        int outputIndex = atoi(strOutputIndex.c_str());
+        int outputIndex;
+        try {
+            outputIndex = std::stoi(strOutputIndex.c_str());
+        } catch (const std::exception& e) {
+            LogPrintf("%s: %s on strOutputIndex\n", __func__, e.what());
+            return false;
+        }
+
         bool found = false;
         BOOST_FOREACH (COutput& out, possibleCoins) {
             if (out.tx->GetHash() == txHash && out.i == outputIndex) {
@@ -439,7 +446,12 @@ vector<COutput> CActiveMasternode::SelectCoinsMasternode()
         uint256 mnTxHash;
         BOOST_FOREACH (CMasternodeConfig::CMasternodeEntry mne, masternodeConfig.getEntries()) {
             mnTxHash.SetHex(mne.getTxHash());
-            COutPoint outpoint = COutPoint(mnTxHash, atoi(mne.getOutputIndex().c_str()));
+
+            int nIndex;
+            if(!mne.castOutputIndex(nIndex))
+                continue;
+
+            COutPoint outpoint = COutPoint(mnTxHash, nIndex);
             confLockedCoins.push_back(outpoint);
             pwalletMain->UnlockCoin(outpoint);
         }

diff --git a/src/masternodeconfig.cpp b/src/masternodeconfig.cpp
@@ -78,3 +78,15 @@ bool CMasternodeConfig::read(std::string& strErr)
     streamConfig.close();
     return true;
 }
+
+bool CMasternodeConfig::CMasternodeEntry::castOutputIndex(int &n)
+{
+    try {
+        n = std::stoi(outputIndex);
+    } catch (const std::exception e) {
+        LogPrintf("%s: %s on getOutputIndex\n", __func__, e.what());
+        return false;
+    }
+
+    return true;
+}
diff --git a/src/masternodeconfig.h b/src/masternodeconfig.h
@@ -53,6 +53,8 @@ class CMasternodeConfig
             return outputIndex;
         }
 
+        bool castOutputIndex(int& n);
+
         void setOutputIndex(const std::string& outputIndex)
         {
             this->outputIndex = outputIndex;

diff --git a/src/qt/masternodelist.cpp b/src/qt/masternodelist.cpp
@@ -131,7 +131,11 @@ void MasternodeList::StartAll(std::string strCommand)
         std::string strError;
         CMasternodeBroadcast mnb;
 
-        CTxIn txin = CTxIn(uint256S(mne.getTxHash()), uint32_t(atoi(mne.getOutputIndex().c_str())));
+        int nIndex;
+        if(!mne.castOutputIndex(nIndex))
+            continue;
+
+        CTxIn txin = CTxIn(uint256S(mne.getTxHash()), uint32_t(nIndex));
         CMasternode* pmn = mnodeman.Find(txin);
 
         if (strCommand == "start-missing" && pmn) continue;
@@ -212,7 +216,11 @@ void MasternodeList::updateMyNodeList(bool fForce)
 
     ui->tableWidgetMasternodes->setSortingEnabled(false);
     BOOST_FOREACH (CMasternodeConfig::CMasternodeEntry mne, masternodeConfig.getEntries()) {
-        CTxIn txin = CTxIn(uint256S(mne.getTxHash()), uint32_t(atoi(mne.getOutputIndex().c_str())));
+        int nIndex;
+        if(!mne.castOutputIndex(nIndex))
+            continue;
+
+        CTxIn txin = CTxIn(uint256S(mne.getTxHash()), uint32_t(nIndex));
         CMasternode* pmn = mnodeman.Find(txin);
 
         updateMyMasternodeInfo(QString::fromStdString(mne.getAlias()), QString::fromStdString(mne.getIp()), pmn);

diff --git a/src/rpcmasternode.cpp b/src/rpcmasternode.cpp
@@ -344,8 +344,10 @@ Value masternode(const Array& params, bool fHelp)
 
         BOOST_FOREACH (CMasternodeConfig::CMasternodeEntry mne, masternodeConfig.getEntries()) {
             std::string errorMessage;
-
-            CTxIn vin = CTxIn(uint256(mne.getTxHash()), uint32_t(atoi(mne.getOutputIndex().c_str())));
+            int nIndex;
+            if(!mne.castOutputIndex(nIndex))
+                continue;
+            CTxIn vin = CTxIn(uint256(mne.getTxHash()), uint32_t(nIndex));
             CMasternode* pmn = mnodeman.Find(vin);
 
             if (strCommand == "start-missing" && pmn) continue;
@@ -393,7 +395,10 @@ Value masternode(const Array& params, bool fHelp)
         Object resultObj;
 
         BOOST_FOREACH (CMasternodeConfig::CMasternodeEntry mne, masternodeConfig.getEntries()) {
-            CTxIn vin = CTxIn(uint256(mne.getTxHash()), uint32_t(atoi(mne.getOutputIndex().c_str())));
+            int nIndex;
+            if(!mne.castOutputIndex(nIndex))
+                continue;
+            CTxIn vin = CTxIn(uint256(mne.getTxHash()), uint32_t(nIndex));
             CMasternode* pmn = mnodeman.Find(vin);
 
             std::string strStatus = pmn ? pmn->Status() : "MISSING";
@@ -440,7 +445,11 @@ Value masternode(const Array& params, bool fHelp)
         int nLast = 10;
 
         if (params.size() >= 2) {
-            nLast = atoi(params[1].get_str());
+            try {
+                nLast = std::stoi(params[1].get_str());
+            } catch (const std::exception& e) {
+                throw runtime_error("Exception on param 2");
+            }
         }
 
         Object obj;
@@ -459,7 +468,11 @@ Value masternode(const Array& params, bool fHelp)
         int nLast = 10;
 
         if (params.size() >= 2) {
-            nLast = atoi(params[1].get_str());
+            try {
+                nLast = std::stoi(params[1].get_str());
+            } catch (const boost::bad_lexical_cast &) {
+                throw runtime_error("Exception on param 2");
+            }
         }
         Object obj;
 

diff --git a/src/util.cpp b/src/util.cpp
@@ -348,8 +348,16 @@ std::string GetArg(const std::string& strArg, const std::string& strDefault)
 
 int64_t GetArg(const std::string& strArg, int64_t nDefault)
 {
-    if (mapArgs.count(strArg))
-        return atoi64(mapArgs[strArg]);
+    if (mapArgs.count(strArg)) {
+        int64_t n;
+        try {
+            n = std::stoi(mapArgs[strArg]);
+        } catch (const std::exception& e) {
+            return nDefault;
+        }
+
+        return n;
+    }
     return nDefault;
 }
 
@@ -358,7 +366,14 @@ bool GetBoolArg(const std::string& strArg, bool fDefault)
     if (mapArgs.count(strArg)) {
         if (mapArgs[strArg].empty())
             return true;
-        return (atoi(mapArgs[strArg]) != 0);
+
+        int n;
+        try {
+            n = std::stoi(mapArgs[strArg]);
+        } catch (const std::exception& e) {
+            return fDefault;
+        }
+        return n;
     }
     return fDefault;
 }

diff --git a/src/wallet.cpp b/src/wallet.cpp
@@ -443,7 +443,14 @@ bool CWallet::GetMasternodeVinAndKeys(CTxIn& txinRet, CPubKey& pubKeyRet, CKey&
 
     // Find specific vin
     uint256 txHash = uint256S(strTxHash);
-    int nOutputIndex = atoi(strOutputIndex.c_str());
+
+    int nOutputIndex;
+    try {
+        nOutputIndex = std::stoi(strOutputIndex.c_str());
+    } catch (const std::exception& e) {
+        LogPrintf("%s: %s on strOutputIndex\n", __func__, e.what());
+        return false;
+    }
 
     BOOST_FOREACH (COutput& out, vPossibleCoins)
         if (out.tx->GetHash() == txHash && out.i == nOutputIndex) // found it!