diff --git a/bbot/modules/gitdumper.py b/bbot/modules/gitdumper.py index 71ce02a338..9fe23a20af 100644 --- a/bbot/modules/gitdumper.py +++ b/bbot/modules/gitdumper.py @@ -201,36 +201,66 @@ async def download_git_packs(self, url, folder): if url_list: await self.download_files(url_list, folder) - async def regex_files(self, regex, folder=Path(), file=Path(), files=[]): + # Skip regex scan for files larger than this — real git ref/object/info + # files are small; oversized is almost always a webserver returning HTML. + _regex_file_max_bytes = 10 * 1024 * 1024 + # Cap download size per file. Real git files are tiny (refs/HEAD <1KB, + # index a few KB) or pack files (10s of MB legit). Anything bigger is + # a misconfigured / malicious server returning an error page. + _download_max_size = "10MB" + # Cap how deep ``download_object`` recursion can go. Real git object + # graphs are shallow (commit → tree → blob, depth 10-20 in practice). + _download_object_max_depth = 100 + + async def regex_files(self, regex, folder=None, file=None, files=()): results = [] - if folder: - if folder.is_dir(): - for file_path in folder.rglob("*"): - if file_path.is_file(): - results.extend(await self.regex_file(regex, file_path)) + if folder is not None and folder.is_dir(): + for file_path in folder.rglob("*"): + if file_path.is_file(): + results.extend(await self.regex_file(regex, file_path)) if files: - for file in files: - results.extend(await self.regex_file(regex, file)) - if file: + for f in files: + results.extend(await self.regex_file(regex, f)) + if file is not None: results.extend(await self.regex_file(regex, file)) return results - async def regex_file(self, regex, file=Path()): - if file.exists() and file.is_file(): - with file.open("r", encoding="utf-8", errors="ignore") as file: - content = file.read() - matches = await self.helpers.re.findall(regex, content) - if matches: - return matches + async def regex_file(self, regex, file=None): + if file is None or not (file.exists() and file.is_file()): + return [] + try: + size = file.stat().st_size + except OSError: + return [] + if size > self._regex_file_max_bytes: + self.debug(f"Skipping regex scan of {file} ({size} bytes)") + return [] + with file.open("r", encoding="utf-8", errors="ignore") as fh: + content = fh.read() + matches = await self.helpers.re.findall(regex, content) + if matches: + return matches return [] - async def download_object(self, object, repo_url, repo_folder): + async def download_object(self, object, repo_url, repo_folder, _seen=None, _depth=0): + if _seen is None: + _seen = set() + # cycle detection — git tree objects can reference each other in + # malformed/malicious repos + if object in _seen: + return + _seen.add(object) + # depth cap — even without cycles, an unbounded tree would burn + # stack frames + memory per frame + if _depth >= self._download_object_max_depth: + self.debug(f"download_object: hit max recursion depth at {object}") + return await self.download_files( [self.helpers.urlparse(self.helpers.urljoin(repo_url, f"objects/{object[:2]}/{object[2:]}"))], repo_folder ) output = await self.git_catfile(object, option="-p", folder=repo_folder) for obj in await self.helpers.re.findall(self.obj_regex, output): - await self.download_object(obj, repo_url, repo_folder) + await self.download_object(obj, repo_url, repo_folder, _seen=_seen, _depth=_depth + 1) async def download_files(self, urls, folder): for url in urls: @@ -240,7 +270,7 @@ async def download_files(self, urls, folder): self.helpers.mkdir(filename.parent) if hash(str(file_url)) not in self.urls_downloaded: self.verbose(f"Downloading {file_url} to {filename}") - await self.helpers.download(file_url, filename=filename, warn=False) + await self.helpers.download(file_url, filename=filename, warn=False, max_size=self._download_max_size) self.urls_downloaded.add(hash(str(file_url))) if any(folder.rglob("*")): return True diff --git a/bbot/test/test_step_1/test_gitdumper_safeguards.py b/bbot/test/test_step_1/test_gitdumper_safeguards.py new file mode 100644 index 0000000000..0ba36e849f --- /dev/null +++ b/bbot/test/test_step_1/test_gitdumper_safeguards.py @@ -0,0 +1,141 @@ +"""Regression tests for ``gitdumper`` safeguards against pathological / malicious +git layouts. + +Each test demonstrates a failure mode that would either leak memory, hang, or +recurse without bound on the unfixed code. +""" + +import pytest + +from ..bbot_fixtures import * # noqa: F401, F403 + + +@pytest.mark.asyncio +async def test_regex_files_does_not_walk_cwd(bbot_scanner, tmp_path, monkeypatch): + """``regex_files(file=foo)`` must scan only ``foo`` — not the cwd.""" + target = tmp_path / "head" + target.write_text("ref: refs/heads/main\n") + + decoy_cwd = tmp_path / "cwd" + decoy_cwd.mkdir() + (decoy_cwd / "decoy.txt").write_text("ref: refs/heads/should_not_match\n") + monkeypatch.chdir(decoy_cwd) + + scan = bbot_scanner("evilcorp.com", modules=["gitdumper"]) + await scan._prep() + try: + gitdumper = scan.modules["gitdumper"] + regex = gitdumper.helpers.re.compile(r"ref: refs/heads/([a-zA-Z\d_-]+)") + results = await gitdumper.regex_files(regex, file=target) + + assert "main" in results, "expected the requested file to be scanned" + assert "should_not_match" not in results, "regex_files walked the cwd in addition to the requested file" + finally: + await scan._cleanup() + + +@pytest.mark.asyncio +async def test_download_files_caps_max_size(bbot_scanner, tmp_path, monkeypatch): + """``download_files`` must pass an explicit ``max_size`` to ``helpers.download``. + + Without it, a misconfigured / malicious server can return up to 500 MB + (the web helper default) per probed git path. Real git refs/info files + are tiny, so capping at a few MB shuts down a whole class of abuse. + """ + scan = bbot_scanner("evilcorp.com", modules=["gitdumper"]) + await scan._prep() + try: + gitdumper = scan.modules["gitdumper"] + seen_kwargs = [] + + async def traced_download(url, **kwargs): + seen_kwargs.append(kwargs) + return None + + monkeypatch.setattr(gitdumper.helpers, "download", traced_download) + + url = gitdumper.helpers.urlparse("http://example.com/.git/HEAD") + await gitdumper.download_files([url], tmp_path) + + assert seen_kwargs, "expected helpers.download to be called" + for call in seen_kwargs: + assert "max_size" in call, "max_size must be passed to helpers.download" + finally: + await scan._cleanup() + + +@pytest.mark.asyncio +async def test_download_object_caps_recursion_depth(bbot_scanner, tmp_path, monkeypatch): + """``download_object`` must bound recursion depth. + + Mocks ``git_catfile`` so every object's output yields a fresh new hash, + creating an infinite chain. With no cap we'd recurse until Python's + stack limit (or the OS kills us). With a cap we stop at a finite depth. + """ + scan = bbot_scanner("evilcorp.com", modules=["gitdumper"]) + await scan._prep() + try: + gitdumper = scan.modules["gitdumper"] + + async def nop_download_files(urls, folder): + return True + + monkeypatch.setattr(gitdumper, "download_files", nop_download_files) + + counter = [0] + + async def fake_catfile(hash_, option="-t", folder=None): + counter[0] += 1 + new_hash = f"{counter[0]:040x}" + return f"object content with {new_hash}" + + monkeypatch.setattr(gitdumper, "git_catfile", fake_catfile) + + await gitdumper.download_object("0" * 40, "http://example.com", tmp_path) + + # If recursion is bounded, counter stays well below Python's stack limit. + # We just need to confirm it didn't spiral out — anything < 500 means + # the cap is in effect. + assert counter[0] < 500, f"download_object recursed {counter[0]} levels — cap not in effect" + finally: + await scan._cleanup() + + +@pytest.mark.asyncio +async def test_download_object_detects_cycles(bbot_scanner, tmp_path, monkeypatch): + """``download_object`` must not re-process an object hash it has + already visited. Malicious or corrupt git repos can have cyclic + references.""" + scan = bbot_scanner("evilcorp.com", modules=["gitdumper"]) + await scan._prep() + try: + gitdumper = scan.modules["gitdumper"] + + async def nop_download_files(urls, folder): + return True + + monkeypatch.setattr(gitdumper, "download_files", nop_download_files) + + hash_a = "a" * 40 + hash_b = "b" * 40 + catfile_calls = [] + + async def fake_catfile(hash_, option="-t", folder=None): + catfile_calls.append(hash_) + if hash_ == hash_a: + return f"references {hash_b}" + if hash_ == hash_b: + return f"references {hash_a}" # cycle back + return "" + + monkeypatch.setattr(gitdumper, "git_catfile", fake_catfile) + + await gitdumper.download_object(hash_a, "http://example.com", tmp_path) + + # With cycle detection: A -> B -> stop (A already seen). 2 catfile calls. + # Without: infinite, eventually RecursionError. + assert len(catfile_calls) <= 2, ( + f"download_object did not detect cycle (called catfile {len(catfile_calls)} times)" + ) + finally: + await scan._cleanup()