diff --git a/src/borg/constants.py b/src/borg/constants.py index 9813a8afe..aaf7d044e 100644 --- a/src/borg/constants.py +++ b/src/borg/constants.py @@ -31,6 +31,11 @@ DEFAULT_MAX_SEGMENT_SIZE = 500 * 1024 * 1024 # the header, and the total size was set to 20 MiB). MAX_DATA_SIZE = 20971479 +# 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 + DEFAULT_SEGMENTS_PER_DIR = 1000 CHUNK_MIN_EXP = 19 # 2**19 == 512kiB diff --git a/src/borg/crypto/key.py b/src/borg/crypto/key.py index aba1f35be..ea0d5e6ae 100644 --- a/src/borg/crypto/key.py +++ b/src/borg/crypto/key.py @@ -24,6 +24,7 @@ from ..helpers import get_keys_dir, get_security_dir from ..helpers import bin_to_hex from ..item import Key, EncryptedKey from ..platform import SaveFile +from .. import remote from .nonces import NonceManager from .low_level import AES, bytes_to_long, bytes_to_int, num_aes_blocks, hmac_sha256, blake2b_256, hkdf_hmac_sha512 @@ -216,9 +217,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()) diff --git a/src/borg/helpers.py b/src/borg/helpers.py index 92c573b2d..95b00185b 100644 --- a/src/borg/helpers.py +++ b/src/borg/helpers.py @@ -351,6 +351,10 @@ class Manifest: prev_ts = self.last_timestamp 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 manifest = ManifestItem( version=1, archives=StableDict(self.archives.get_raw_dict()), diff --git a/src/borg/remote.py b/src/borg/remote.py index 670179a44..11a2a251f 100644 --- a/src/borg/remote.py +++ b/src/borg/remote.py @@ -20,6 +20,7 @@ import msgpack from . import __version__ from .compress import LZ4 +from .constants import * # NOQA from .helpers import Error, IntegrityError from .helpers import bin_to_hex from .helpers import get_home_dir @@ -28,6 +29,7 @@ from .helpers import replace_placeholders from .helpers import sysinfo from .helpers import format_file_size from .helpers import truncate_and_unlink +from .helpers import StableDict from .logger import create_logger, setup_logging from .repository import Repository, MAX_OBJECT_SIZE, LIST_SCAN_LIMIT from .version import parse_version, format_version @@ -81,8 +83,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) diff --git a/src/borg/testsuite/key.py b/src/borg/testsuite/key.py index df7f81e5e..6a7a6c8d7 100644 --- a/src/borg/testsuite/key.py +++ b/src/borg/testsuite/key.py @@ -331,7 +331,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):