Manifest: use limited unpacker

(cherry picked from commit 6c2c51939d30eaab7fc9791f486b54f94de50659)

borg/helpers.py

@@ -39,6 +39,12 @@ import msgpack.fallback
 
 import socket
 
+
+# to use a safe, limited unpacker, we need to set a upper limit to the archive count in the manifest.
+# this does not mean that you can always really reach that number, because it also needs to be less than
+# MAX_DATA_SIZE or it will trigger the check for that.
+MAX_ARCHIVES = 400000
+
 # return codes returned by borg command
 # when borg is killed by signal N, rc = 128 + N
 EXIT_SUCCESS = 0  # everything done, no problems
@@ -254,6 +260,10 @@ class Manifest:
             prev_ts = datetime.strptime(self.timestamp, "%Y-%m-%dT%H:%M:%S.%f")
             incremented = (prev_ts + timedelta(microseconds=1)).isoformat()
             self.timestamp = max(incremented, datetime.utcnow().isoformat())
+        # include checks for limits as enforced by limited unpacker (used by load())
+        assert len(self.archives) <= MAX_ARCHIVES
+        assert all(len(name) <= 255 for name in self.archives)
+        assert len(self.item_keys) <= 100
         m = {
             'version': 1,
             'archives': StableDict((name, StableDict(archive)) for name, archive in self.archives.items()),

borg/key.py

@@ -16,6 +16,8 @@ logger = create_logger()
 from .crypto import AES, bytes_to_long, long_to_bytes, bytes_to_int, num_aes_blocks
 from .crypto import hkdf_hmac_sha512
 from .compress import Compressor, CNONE
+from . import remote
+
 
 PREFIX = b'\0' * 8
 
@@ -155,9 +157,9 @@ class KeyBase:
             logger.warning('Manifest authentication DISABLED.')
             tam_required = False
         data = bytearray(data)
-        # Since we don't trust these bytes we use the slower Python unpacker,
-        # which is assumed to have a lower probability of security issues.
-        unpacked = msgpack.fallback.unpackb(data, object_hook=StableDict, unicode_errors='surrogateescape')
+        unpacker = remote.get_limited_unpacker('manifest')
+        unpacker.feed(data)
+        unpacked = unpacker.unpack()
         if b'tam' not in unpacked:
             if tam_required:
                 raise TAMRequiredError(self.repository._location.canonical_path())

borg/remote.py

@@ -15,6 +15,8 @@ from . import __version__
 from .helpers import Error, IntegrityError, sysinfo
 from .helpers import replace_placeholders
 from .helpers import bin_to_hex
+from .helpers import StableDict
+from .helpers import MAX_ARCHIVES
 from .repository import Repository, LIST_SCAN_LIMIT, MAX_OBJECT_SIZE
 from .logger import create_logger
 
@@ -64,8 +66,16 @@ def get_limited_unpacker(kind):
         args.update(dict(max_array_len=LIST_SCAN_LIMIT,  # result list from repo.list() / .scan()
                          max_map_len=100,  # misc. result dicts
                          ))
+    elif kind == 'manifest':
+        args.update(dict(use_list=True,  # default value
+                         max_array_len=100,  # ITEM_KEYS ~= 22
+                         max_map_len=MAX_ARCHIVES,  # list of archives
+                         max_str_len=255,  # archive name
+                         object_hook=StableDict,
+                         unicode_errors='surrogateescape',
+                         ))
     else:
-        raise ValueError('kind must be "server" or "client"')
+        raise ValueError('kind must be "server", "client" or "manifest"')
     return msgpack.Unpacker(**args)
 
 

borg/testsuite/key.py

@@ -123,7 +123,7 @@ class TestTAM:
             key.unpack_and_verify_manifest(blob)
 
         blob = b'\xc1\xc1\xc1'
-        with pytest.raises(msgpack.UnpackException):
+        with pytest.raises((ValueError, msgpack.UnpackException)):
             key.unpack_and_verify_manifest(blob)
 
     def test_missing_when_required(self, key):