Merge #14377: check that a separator is found for psbt inputs, outputs, and global map

4fb3388db95f408566e43ebb9736842cfbff0a7d check that a separator is found for psbt inputs, outputs, and global map (Andrew Chow)

Pull request description:

  Currently it doesn't make sure that a separator was found so PSBTs missing a trailing separator would still pass. This fixes that and adds a test case for it.

  It really only makes sense to check for the separator for the output maps as if an input or global map was missing a separator, the fields following it would be interpreted as belonging to the previous input or global map. However I have added the check for those two anyways to be consistent.

Tree-SHA512: 50c0c08e201ba02494b369a4d36ddb73e6634eb5a4e4e201c4ef38fd2dbeea2c642b8a04d50c91615da61ecbfade37309e47431368f4b1064539c42015766b50

1 files changed, 27 insertions, 3 deletions

diff --git a/src/script/sign.h b/src/script/sign.h
index 689501269d..d47aada17d 100644
--- a/src/script/sign.h
+++ b/src/script/sign.h
@@ -302,6 +302,7 @@ struct PSBTInput
     template <typename Stream>
     inline void Unserialize(Stream& s) {
         // Read loop
+        bool found_sep = false;
         while(!s.empty()) {
             // Read
             std::vector<unsigned char> key;
@@ -309,7 +310,10 @@ struct PSBTInput
 
             // the key is empty if that was actually a separator byte
             // This is a special case for key lengths 0 as those are not allowed (except for separator)
-            if (key.empty()) return;
+            if (key.empty()) {
+                found_sep = true;
+                break;
+            }
 
             // First byte of key is the type
             unsigned char type = key[0];
@@ -424,6 +428,10 @@ struct PSBTInput
                     break;
             }
         }
+
+        if (!found_sep) {
+            throw std::ios_base::failure("Separator is missing at the end of an input map");
+        }
     }
 
     template <typename Stream>
@@ -477,6 +485,7 @@ struct PSBTOutput
     template <typename Stream>
     inline void Unserialize(Stream& s) {
         // Read loop
+        bool found_sep = false;
         while(!s.empty()) {
             // Read
             std::vector<unsigned char> key;
@@ -484,7 +493,10 @@ struct PSBTOutput
 
             // the key is empty if that was actually a separator byte
             // This is a special case for key lengths 0 as those are not allowed (except for separator)
-            if (key.empty()) return;
+            if (key.empty()) {
+                found_sep = true;
+                break;
+            }
 
             // First byte of key is the type
             unsigned char type = key[0];
@@ -529,6 +541,10 @@ struct PSBTOutput
                 }
             }
         }
+
+        if (!found_sep) {
+            throw std::ios_base::failure("Separator is missing at the end of an output map");
+        }
     }
 
     template <typename Stream>
@@ -604,6 +620,7 @@ struct PartiallySignedTransaction
         }
 
         // Read global data
+        bool found_sep = false;
         while(!s.empty()) {
             // Read
             std::vector<unsigned char> key;
@@ -611,7 +628,10 @@ struct PartiallySignedTransaction
 
             // the key is empty if that was actually a separator byte
             // This is a special case for key lengths 0 as those are not allowed (except for separator)
-            if (key.empty()) break;
+            if (key.empty()) {
+                found_sep = true;
+                break;
+            }
 
             // First byte of key is the type
             unsigned char type = key[0];
@@ -651,6 +671,10 @@ struct PartiallySignedTransaction
             }
         }
 
+        if (!found_sep) {
+            throw std::ios_base::failure("Separator is missing at the end of the global map");
+        }
+
         // Make sure that we got an unsigned tx
         if (!tx) {
             throw std::ios_base::failure("No unsigned transcation was provided");