Skip to content

fix(sitemap): bound sitemap decompression size and nesting depth to prevent DoS#2012

Open
vdusek wants to merge 2 commits into
masterfrom
worktree-fix-remote-content-dos-via-sitemaps
Open

fix(sitemap): bound sitemap decompression size and nesting depth to prevent DoS#2012
vdusek wants to merge 2 commits into
masterfrom
worktree-fix-remote-content-dos-via-sitemaps

Conversation

@vdusek

@vdusek vdusek commented Jul 2, 2026

Copy link
Copy Markdown
Collaborator

Problem

The sitemap parsing pipeline processes content from arbitrary crawled hosts, which are untrusted. Two vectors let a malicious host exhaust crawler memory or trap it in unbounded work:

  • Gzip decompression bomb. parse_sitemap decompressed gzipped sitemaps with no output cap, so a ~50 KB payload could inflate to gigabytes.
  • Unbounded nested-sitemap recursion. parse_sitemap defaulted max_depth to float('inf') with only exact-URL dedup, so a chain of unique nested-sitemap URLs recursed forever.

Changes

  • Gzip decompression bomb. Decompression and parsing now run in bounded pieces capped at MAX_SITEMAP_SIZE (50 MB, the sitemap-protocol limit). The read loop also stops once the gzip stream ends (otherwise trailing bytes accumulate unbounded in the decompressor's unused_data) and once too many raw bytes are read (guarding a stream that produces little or no output). On a repro, peak memory dropped from ~680 MB to ~15 MB.
  • Unbounded nested-sitemap recursion. The default is now DEFAULT_MAX_DEPTH = 10. SitemapRequestLoader — the main consumer — drives nesting itself (it passes max_depth=0), so it now tracks per-sitemap depth in its persisted state and stops following chains past the same limit.

Out of scope

A third listed vector, unbounded robots.txt reads, is intentionally not addressed here. Capping it requires switching RobotsTxtFile.load from send_request to streaming, which regresses CurlImpersonateHttpClient (a multi-second stall in the crawl pipeline that fails test_respect_robots_txt_with_problematic_links[curl]). Since send_request reads the whole body eagerly and there is no size-limited read primitive in the HTTP client layer, there is no safe non-streaming cap. It is the lowest-severity vector (no compression amplification — the attacker must transmit the full bytes), so it is left as a follow-up: add a size-limited read to the HttpClient streaming layer, then cap robots.txt there.

Tests

Adds regression tests for gzip-bomb truncation, stop-after-gzip-member, parse_sitemap depth, and loader nested-chain depth.

@vdusek vdusek added t-tooling Issues with this label are in the ownership of the tooling team. adhoc Ad-hoc unplanned task added during the sprint. labels Jul 2, 2026
@vdusek vdusek self-assigned this Jul 2, 2026
@codecov

codecov Bot commented Jul 2, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 93.37%. Comparing base (7780e78) to head (c6f09d5).
⚠️ Report is 57 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #2012      +/-   ##
==========================================
+ Coverage   93.35%   93.37%   +0.02%     
==========================================
  Files         179      179              
  Lines       12482    12531      +49     
==========================================
+ Hits        11652    11701      +49     
  Misses        830      830              
Flag Coverage Δ
unit 93.37% <100.00%> (+0.02%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@vdusek vdusek changed the title fix: bound sitemap decompression size and nesting depth to prevent DoS fix(sitemap): bound sitemap decompression size and nesting depth to prevent DoS Jul 10, 2026
@vdusek vdusek requested a review from Copilot July 14, 2026 12:22

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR hardens the sitemap parsing pipeline against untrusted-host DoS vectors by bounding gzip decompression output and nested-sitemap traversal depth, and adds regression tests to prevent regressions in these safeguards.

Changes:

  • Add a decompression/parsing loop that processes sitemap content in bounded pieces and truncates processing beyond MAX_SITEMAP_SIZE.
  • Introduce DEFAULT_MAX_DEPTH = 10 for parse_sitemap and enforce nested-sitemap depth limiting in SitemapRequestLoader with persisted per-sitemap depth tracking.
  • Add unit tests covering gzip bomb truncation, stopping after gzip member end, and depth-bounded nested-sitemap chains.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

File Description
src/crawlee/_utils/sitemap.py Adds size-bounded streaming decompression/parsing and sets a finite default nested-sitemap max depth.
src/crawlee/request_loaders/_sitemap_request_loader.py Tracks and enforces per-sitemap nesting depth in persisted loader state to prevent infinite unique-URL sitemap chains.
tests/unit/_utils/test_sitemap.py Adds regression tests for truncation behavior, stop-after-gzip-member, and parse_sitemap depth bounding.
tests/unit/request_loaders/test_sitemap_request_loader.py Adds a loader-level regression test ensuring nested sitemap chains are bounded by the default max depth.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/crawlee/_utils/sitemap.py
@vdusek vdusek marked this pull request as ready for review July 14, 2026 12:52
@vdusek vdusek requested review from Mantisus and Pijukatel July 14, 2026 12:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

adhoc Ad-hoc unplanned task added during the sprint. t-tooling Issues with this label are in the ownership of the tooling team.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants