diff --git a/docs/changes.rst b/docs/changes.rst index 928a9c2e2..7da0c2f1d 100644 --- a/docs/changes.rst +++ b/docs/changes.rst @@ -134,6 +134,9 @@ Security fixes: - A flaw in the cryptographic authentication scheme in Borg allowed an attacker to spoof the manifest. See :ref:`tam_vuln` above for the steps you should take. +- borg check: When rebuilding the manifest (which should only be needed very rarely) + duplicate archive names would be handled on a "first come first serve" basis, allowing + an attacker to apparently replace archives. Bug fixes: diff --git a/src/borg/archive.py b/src/borg/archive.py index 3793e7e7f..90759dae8 100644 --- a/src/borg/archive.py +++ b/src/borg/archive.py @@ -1197,8 +1197,18 @@ class ArchiveChecker: continue if valid_archive(archive): archive = ArchiveItem(internal_dict=archive) - logger.info('Found archive %s', archive.name) - manifest.archives[archive.name] = (chunk_id, archive.time) + name = archive.name + logger.info('Found archive %s', name) + if name in manifest.archives: + i = 1 + while True: + new_name = '%s.%d' % (name, i) + if new_name not in manifest.archives: + break + i += 1 + logger.warning('Duplicate archive name %s, storing as %s', name, new_name) + name = new_name + manifest.archives[name] = (chunk_id, archive.time) logger.info('Manifest rebuild complete.') return manifest diff --git a/src/borg/testsuite/archiver.py b/src/borg/testsuite/archiver.py index a9c2c7ba2..5637446cc 100644 --- a/src/borg/testsuite/archiver.py +++ b/src/borg/testsuite/archiver.py @@ -2180,6 +2180,33 @@ class ArchiverCheckTestCase(ArchiverTestCaseBase): self.assert_in('archive2', output) self.cmd('check', self.repository_location, exit_code=0) + def test_manifest_rebuild_duplicate_archive(self): + archive, repository = self.open_archive('archive1') + key = archive.key + with repository: + manifest = repository.get(Manifest.MANIFEST_ID) + corrupted_manifest = manifest + b'corrupted!' + repository.put(Manifest.MANIFEST_ID, corrupted_manifest) + + archive = msgpack.packb({ + 'cmdline': [], + 'items': [], + 'hostname': 'foo', + 'username': 'bar', + 'name': 'archive1', + 'time': '2016-12-15T18:49:51.849711', + 'version': 1, + }) + archive_id = key.id_hash(archive) + repository.put(archive_id, key.encrypt(Chunk(archive))) + repository.commit() + self.cmd('check', self.repository_location, exit_code=1) + self.cmd('check', '--repair', self.repository_location, exit_code=0) + output = self.cmd('list', self.repository_location) + self.assert_in('archive1', output) + self.assert_in('archive1.1', output) + self.assert_in('archive2', output) + def test_extra_chunks(self): self.cmd('check', self.repository_location, exit_code=0) with Repository(self.repository_location, exclusive=True) as repository: