Issue #12848: The pure Python pickle implementation now treats object lengths as unsigned 32-bit integers, like the C implementation does.

Patch by Serhiy Storchaka.

3 files changed, 31 insertions, 14 deletions

diff --git a/Lib/pickle.py b/Lib/pickle.py
index 9e65368..e81a379 100644
--- a/Lib/pickle.py
+++ b/Lib/pickle.py
@@ -263,7 +263,7 @@ class _Pickler:
             if i < 256:
                 return BINPUT + bytes([i])
             else:
-                return LONG_BINPUT + pack("<i", i)
+                return LONG_BINPUT + pack("<I", i)
 
         return PUT + repr(i).encode("ascii") + b'\n'
 
@@ -273,7 +273,7 @@ class _Pickler:
             if i < 256:
                 return BINGET + bytes([i])
             else:
-                return LONG_BINGET + pack("<i", i)
+                return LONG_BINGET + pack("<I", i)
 
         return GET + repr(i).encode("ascii") + b'\n'
 
@@ -503,7 +503,7 @@ class _Pickler:
         if n < 256:
             self.write(SHORT_BINBYTES + bytes([n]) + bytes(obj))
         else:
-            self.write(BINBYTES + pack("<i", n) + bytes(obj))
+            self.write(BINBYTES + pack("<I", n) + bytes(obj))
         self.memoize(obj)
     dispatch[bytes] = save_bytes
 
@@ -511,7 +511,7 @@ class _Pickler:
         if self.bin:
             encoded = obj.encode('utf-8', 'surrogatepass')
             n = len(encoded)
-            self.write(BINUNICODE + pack("<i", n) + encoded)
+            self.write(BINUNICODE + pack("<I", n) + encoded)
         else:
             obj = obj.replace("\\", "\\u005c")
             obj = obj.replace("\n", "\\u000a")
@@ -931,6 +931,9 @@ class _Unpickler:
 
     def load_long4(self):
         n = mloads(b'i' + self.read(4))
+        if n < 0:
+            # Corrupt or hostile pickle -- we never write one like this
+            raise UnpicklingError("LONG pickle has negative byte count");
         data = self.read(n)
         self.append(decode_long(data))
     dispatch[LONG4[0]] = load_long4
@@ -959,14 +962,19 @@ class _Unpickler:
     dispatch[STRING[0]] = load_string
 
     def load_binstring(self):
+        # Deprecated BINSTRING uses signed 32-bit length
         len = mloads(b'i' + self.read(4))
+        if len < 0:
+            raise UnpicklingError("BINSTRING pickle has negative byte count");
         data = self.read(len)
         value = str(data, self.encoding, self.errors)
         self.append(value)
     dispatch[BINSTRING[0]] = load_binstring
 
-    def load_binbytes(self):
-        len = mloads(b'i' + self.read(4))
+    def load_binbytes(self, unpack=struct.unpack, maxsize=sys.maxsize):
+        len, = unpack('<I', self.read(4))
+        if len > maxsize:
+            raise UnpicklingError("BINBYTES exceeds system's maximum size of %d bytes" % maxsize);
         self.append(self.read(len))
     dispatch[BINBYTES[0]] = load_binbytes
 
@@ -974,8 +982,10 @@ class _Unpickler:
         self.append(str(self.readline()[:-1], 'raw-unicode-escape'))
     dispatch[UNICODE[0]] = load_unicode
 
-    def load_binunicode(self):
-        len = mloads(b'i' + self.read(4))
+    def load_binunicode(self, unpack=struct.unpack, maxsize=sys.maxsize):
+        len, = unpack('<I', self.read(4))
+        if len > maxsize:
+            raise UnpicklingError("BINUNICODE exceeds system's maximum size of %d bytes" % maxsize);
         self.append(str(self.read(len), 'utf-8', 'surrogatepass'))
     dispatch[BINUNICODE[0]] = load_binunicode
 
@@ -1106,6 +1116,9 @@ class _Unpickler:
             return
         key = _inverted_registry.get(code)
         if not key:
+            if code <= 0: # note that 0 is forbidden
+                # Corrupt or hostile pickle.
+                raise UnpicklingError("EXT specifies code <= 0");
             raise ValueError("unregistered extension code %d" % code)
         obj = self.find_class(*key)
         _extension_cache[code] = obj
@@ -1159,8 +1172,8 @@ class _Unpickler:
         self.append(self.memo[i])
     dispatch[BINGET[0]] = load_binget
 
-    def load_long_binget(self):
-        i = mloads(b'i' + self.read(4))
+    def load_long_binget(self, unpack=struct.unpack):
+        i, = unpack('<I', self.read(4))
         self.append(self.memo[i])
     dispatch[LONG_BINGET[0]] = load_long_binget
 
@@ -1178,9 +1191,9 @@ class _Unpickler:
         self.memo[i] = self.stack[-1]
     dispatch[BINPUT[0]] = load_binput
 
-    def load_long_binput(self):
-        i = mloads(b'i' + self.read(4))
-        if i < 0:
+    def load_long_binput(self, unpack=struct.unpack, maxsize=sys.maxsize):
+        i, = unpack('<I', self.read(4))
+        if i > maxsize:
             raise ValueError("negative LONG_BINPUT argument")
         self.memo[i] = self.stack[-1]
     dispatch[LONG_BINPUT[0]] = load_long_binput
diff --git a/Misc/NEWS b/Misc/NEWS
index b3dacd2..f17cfea 100644
--- a/Misc/NEWS
+++ b/Misc/NEWS
@@ -94,6 +94,10 @@ Core and Builtins
 Library
 -------
 
+- Issue #12848: The pure Python pickle implementation now treats object
+  lengths as unsigned 32-bit integers, like the C implementation does.
+  Patch by Serhiy Storchaka.
+
 - Issue #16408: Fix file descriptors not being closed in error conditions
   in the zipfile module.  Patch by Serhiy Storchaka.
 
diff --git a/Modules/_pickle.c b/Modules/_pickle.c
index 5ba5599..c0b5cf5 100644
--- a/Modules/_pickle.c
+++ b/Modules/_pickle.c
@@ -1589,7 +1589,7 @@ save_long(PicklerObject *self, PyObject *obj)
          * byte at the start, and cut it back later if possible.
          */
         nbytes = (nbits >> 3) + 1;
-        if (nbytes > INT_MAX) {
+        if (nbytes > 0x7fffffffL) {
             PyErr_SetString(PyExc_OverflowError,
                             "long too large to pickle");
             goto error;