CVE-2019-16930: Improper Handling of Exceptional Conditions (CWE-755)
value: Zcashd in Zcash before 2.0.7-3 allows discovery of the IP address of a full node that owns a shielded address, related to mishandling of exceptions during deserialization of note plaintexts. This affects anyone who has disclosed their zaddr to a third party.
CWE description: The product does not handle or incorrectly handles an exceptional condition.
CVSS v3 base score: 5.3
Published: 2019-09-28T22:15Z
Repository: https://github.com/zcash/zcash
Language: C++
Files changed: src/zcash/Note.cpp
Commit date: 2019-09-19 22:29:24 +0100
Commit message: Ignore exceptions when deserializing note plaintexts

Diff: diff --git a/src/zcash/Note.cpp b/src/zcash/Note.cpp
index ee8f7b641..23210c784 100644
--- a/src/zcash/Note.cpp
+++ b/src/zcash/Note.cpp
@@ -171,19 +171,25 @@ boost::optional<SaplingOutgoingPlaintext> SaplingOutgoingPlaintext::decrypt(
     if (!pt) {
         return boost::none;
     }
 
     // Deserialize from the plaintext
-    CDataStream ss(SER_NETWORK, PROTOCOL_VERSION);
-    ss << pt.get();
+    try {
+        CDataStream ss(SER_NETWORK, PROTOCOL_VERSION);
+        ss << pt.get();
 
-    SaplingOutgoingPlaintext ret;
-    ss >> ret;
+        SaplingOutgoingPlaintext ret;
+        ss >> ret;
 
-    assert(ss.size() == 0);
+        assert(ss.size() == 0);
 
-    return ret;
+        return ret;
+    } catch (const boost::thread_interrupted&) {
+        throw;
+    } catch (...) {
+        return boost::none;
+    }
 }
 
 boost::optional<SaplingNotePlaintext> SaplingNotePlaintext::decrypt(
     const SaplingEncCiphertext &ciphertext,
     const uint256 &ivk,
@@ -195,17 +201,21 @@ boost::optional<SaplingNotePlaintext> SaplingNotePlaintext::decrypt(
     if (!pt) {
         return boost::none;
     }
 
     // Deserialize from the plaintext
-    CDataStream ss(SER_NETWORK, PROTOCOL_VERSION);
-    ss << pt.get();
-
     SaplingNotePlaintext ret;
-    ss >> ret;
-
-    assert(ss.size() == 0);
+    try {
+        CDataStream ss(SER_NETWORK, PROTOCOL_VERSION);
+        ss << pt.get();
+        ss >> ret;
+        assert(ss.size() == 0);
+    } catch (const boost::thread_interrupted&) {
+        throw;
+    } catch (...) {
+        return boost::none;
+    }
 
     uint256 pk_d;
     if (!librustzcash_ivk_to_pkd(ivk.begin(), ret.d.data(), pk_d.begin())) {
         return boost::none;
     }
@@ -241,15 +251,21 @@ boost::optional<SaplingNotePlaintext> SaplingNotePlaintext::decrypt(
     if (!pt) {
         return boost::none;
     }
 
     // Deserialize from the plaintext
-    CDataStream ss(SER_NETWORK, PROTOCOL_VERSION);
-    ss << pt.get();
-
     SaplingNotePlaintext ret;
-    ss >> ret;
+    try {
+        CDataStream ss(SER_NETWORK, PROTOCOL_VERSION);
+        ss << pt.get();
+        ss >> ret;
+        assert(ss.size() == 0);
+    } catch (const boost::thread_interrupted&) {
+        throw;
+    } catch (...) {
+        return boost::none;
+    }
 
     uint256 cmu_expected;
     if (!librustzcash_sapling_compute_cm(
         ret.d.data(),
         pk_d.begin(),
@@ -263,12 +279,10 @@ boost::optional<SaplingNotePlaintext> SaplingNotePlaintext::decrypt(
 
     if (cmu_expected != cmu) {
         return boost::none;
     }
 
-    assert(ss.size() == 0);
-
     return ret;
 }
 
 boost::optional<SaplingNotePlaintextEncryptionResult> SaplingNotePlaintext::encrypt(const uint256& pk_d) const
 {

Vulnerable code:     CDataStream ss(SER_NETWORK, PROTOCOL_VERSION);
    ss << pt.get();
    SaplingOutgoingPlaintext ret;
    ss >> ret;
    assert(ss.size() == 0);
    return ret;
    CDataStream ss(SER_NETWORK, PROTOCOL_VERSION);
    ss << pt.get();

    ss >> ret;

    assert(ss.size() == 0);
    CDataStream ss(SER_NETWORK, PROTOCOL_VERSION);
    ss << pt.get();

    ss >> ret;
    assert(ss.size() == 0);

Fixed code:     try {
        CDataStream ss(SER_NETWORK, PROTOCOL_VERSION);
        ss << pt.get();
        SaplingOutgoingPlaintext ret;
        ss >> ret;
        assert(ss.size() == 0);
        return ret;
    } catch (const boost::thread_interrupted&) {
        throw;
    } catch (...) {
        return boost::none;
    }
    try {
        CDataStream ss(SER_NETWORK, PROTOCOL_VERSION);
        ss << pt.get();
        ss >> ret;
        assert(ss.size() == 0);
    } catch (const boost::thread_interrupted&) {
        throw;
    } catch (...) {
        return boost::none;
    }
    try {
        CDataStream ss(SER_NETWORK, PROTOCOL_VERSION);
        ss << pt.get();
        ss >> ret;
        assert(ss.size() == 0);
    } catch (const boost::thread_interrupted&) {
        throw;
    } catch (...) {
        return boost::none;
    }