From 29613938d51509208ef33891f0b059218a4b3ead Mon Sep 17 00:00:00 2001 From: Marian Beermann Date: Fri, 10 Mar 2017 18:48:55 +0100 Subject: [PATCH] borg rpc: use limited msgpack.Unpacker, fixes #2139 we do not trust the remote, so we are careful unpacking its responses. the remote could return manipulated msgpack data that announces e.g. a huge array or map or string. the local would then need to allocate huge amounts of RAM in expectation of that data (no matter whether really that much is coming or not). by using limits in the Unpacker, a ValueError will be raised if unexpected amounts of data shall get unpacked. memory DoS will be avoided. # Conflicts: # borg/archiver.py # src/borg/archive.py # src/borg/remote.py # src/borg/repository.py --- borg/archive.py | 4 ++-- borg/archiver.py | 4 ++-- borg/remote.py | 27 ++++++++++++++++++++++++--- borg/repository.py | 2 ++ 4 files changed, 30 insertions(+), 7 deletions(-) diff --git a/borg/archive.py b/borg/archive.py index 4d436c860..df1caa6ff 100644 --- a/borg/archive.py +++ b/borg/archive.py @@ -23,7 +23,7 @@ from .helpers import Error, uid2user, user2uid, gid2group, group2gid, bin_to_hex from .platform import acl_get, acl_set from .chunker import Chunker from .hashindex import ChunkIndex -from .repository import Repository +from .repository import Repository, LIST_SCAN_LIMIT import msgpack @@ -882,7 +882,7 @@ class ArchiveChecker: self.chunks = ChunkIndex(capacity) marker = None while True: - result = self.repository.list(limit=10000, marker=marker) + result = self.repository.list(limit=LIST_SCAN_LIMIT, marker=marker) if not result: break marker = result[-1] diff --git a/borg/archiver.py b/borg/archiver.py index 337c03fd6..420cfb32f 100644 --- a/borg/archiver.py +++ b/borg/archiver.py @@ -29,7 +29,7 @@ from .logger import create_logger, setup_logging logger = create_logger() from .compress import Compressor from .upgrader import AtticRepositoryUpgrader, BorgRepositoryUpgrader -from .repository import Repository +from .repository import Repository, LIST_SCAN_LIMIT from .cache import Cache from .key import key_creator, tam_required_file, tam_required, RepoKey, PassphraseKey from .keymanager import KeyManager @@ -813,7 +813,7 @@ class Archiver: marker = None i = 0 while True: - result = repository.list(limit=10000, marker=marker) + result = repository.list(limit=LIST_SCAN_LIMIT, marker=marker) if not result: break marker = result[-1] diff --git a/borg/remote.py b/borg/remote.py index 30291cba3..f0dbc9cf5 100644 --- a/borg/remote.py +++ b/borg/remote.py @@ -15,7 +15,7 @@ from . import __version__ from .helpers import Error, IntegrityError, sysinfo from .helpers import replace_placeholders from .helpers import bin_to_hex -from .repository import Repository +from .repository import Repository, LIST_SCAN_LIMIT, MAX_OBJECT_SIZE from .logger import create_logger import msgpack @@ -46,6 +46,27 @@ def os_write(fd, data): return amount +def get_limited_unpacker(kind): + """return a limited Unpacker because we should not trust msgpack data received from remote""" + args = dict(use_list=False, # return tuples, not lists + max_bin_len=0, # not used + max_ext_len=0, # not used + max_buffer_size=3 * max(BUFSIZE, MAX_OBJECT_SIZE), + max_str_len=MAX_OBJECT_SIZE, # a chunk or other repo object + ) + if kind == 'server': + args.update(dict(max_array_len=100, # misc. cmd tuples + max_map_len=100, # misc. cmd dicts + )) + elif kind == 'client': + args.update(dict(max_array_len=LIST_SCAN_LIMIT, # result list from repo.list() / .scan() + max_map_len=100, # misc. result dicts + )) + else: + raise ValueError('kind must be "server" or "client"') + return msgpack.Unpacker(**args) + + class ConnectionClosed(Error): """Connection closed by remote host""" @@ -115,7 +136,7 @@ class RepositoryServer: # pragma: no cover # Make stderr blocking fl = fcntl.fcntl(stderr_fd, fcntl.F_GETFL) fcntl.fcntl(stderr_fd, fcntl.F_SETFL, fl & ~os.O_NONBLOCK) - unpacker = msgpack.Unpacker(use_list=False) + unpacker = get_limited_unpacker('server') while True: r, w, es = select.select([stdin_fd], [], [], 10) if r: @@ -205,7 +226,7 @@ class RemoteRepository: self.cache = {} self.ignore_responses = set() self.responses = {} - self.unpacker = msgpack.Unpacker(use_list=False) + self.unpacker = get_limited_unpacker('client') self.p = None testing = location.host == '__testsuite__' borg_cmd = self.borg_cmd(args, testing) diff --git a/borg/repository.py b/borg/repository.py index e85c33d6f..71954d32c 100644 --- a/borg/repository.py +++ b/borg/repository.py @@ -26,6 +26,8 @@ TAG_PUT = 0 TAG_DELETE = 1 TAG_COMMIT = 2 +LIST_SCAN_LIMIT = 10000 # repo.list() / .scan() result count limit the borg client uses + class Repository: """Filesystem based transactional key value store