Skip to content

mime: only allow 40 levels of nesting#21384

Closed
bagder wants to merge 5 commits into
masterfrom
bagder/mime-nested-max
Closed

mime: only allow 40 levels of nesting#21384
bagder wants to merge 5 commits into
masterfrom
bagder/mime-nested-max

Conversation

@bagder
Copy link
Copy Markdown
Member

@bagder bagder commented Apr 20, 2026

To avoid problems when doing insane things.

To avoid problems when doing insane things.
@bagder bagder added the MIME label Apr 20, 2026
@bagder bagder marked this pull request as ready for review April 20, 2026 08:29
@bagder bagder requested a review from Copilot April 20, 2026 08:35
Copy link
Copy Markdown

Copilot AI left a comment

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 aims to limit MIME multipart nesting depth during MIME readback to prevent pathological/“insane” deeply-nested structures from causing problems in curl’s MIME streaming logic.

Changes:

  • Introduces a MAX_MIME_LEVELS constant (40) in lib/mime.c.
  • Threads a maxlevel parameter through the MIME readback call stack.
  • Adds guards intended to stop processing once the nesting limit is exceeded.
Comments suppressed due to low confidence (2)

lib/mime.c:674

  • The maxlevel guard here is problematic: readback_bytes() is not part of the multipart recursion, yet it decrements maxlevel and returns 0 when it hits the limit. Returning 0 is interpreted as “segment finished” and will advance the state machine, potentially truncating headers/boundaries silently instead of failing. Consider removing maxlevel from this helper entirely, or at least change the guard to a non-underflowing check (e.g., if(!maxlevel) …) and propagate an error (READ_ERROR) when the nesting limit is exceeded.
    sz = numbytes - offset;
    bytes += offset;

lib/mime.c:929

  • The nesting limit accounting doesn’t match “40 levels” as implemented: maxlevel is decremented in multiple stack frames (readback_part, read_part_content, mime_subparts_read, etc.), so each multipart nesting level consumes several counts. This likely rejects much shallower nesting than intended. Consider tracking multipart depth explicitly (increment only when entering MIMEKIND_MULTIPART) or treating maxlevel as “levels remaining” and decrementing only on the recursive descent call site.
    curl_mimepart *part = mime->state.ptr;
    switch(mime->state.state) {
    case MIMESTATE_BEGIN:

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

Comment thread lib/mime.c
Comment thread lib/mime.c Outdated
@icing
Copy link
Copy Markdown
Contributor

icing commented Apr 20, 2026

While the clanker shows its lack of overall understanding, I'd find the change better readable when maxlevel becomes call_depth and is incremented when the parameter is passed. Checks then call_depth against MAX_DEPTH.

Just a matter of taste.

@bagder
Copy link
Copy Markdown
Member Author

bagder commented Apr 20, 2026

I might do that. I'm also trying out some other approaches...

Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

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


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

Comment thread lib/mime.c Outdated
@bagder
Copy link
Copy Markdown
Member Author

bagder commented Apr 20, 2026

augment review

@augmentcode
Copy link
Copy Markdown

augmentcode Bot commented Apr 20, 2026

🤖 Augment PR Summary

Summary: Adds a hard cap to MIME traversal to prevent excessively deep multipart nesting from causing problems.

Changes:

  • Introduced MAX_MIME_LEVELS (40) and threaded a call_depth parameter through MIME readback helpers.
  • Returns READ_ERROR when the depth limit is exceeded during readback/subpart processing.

Technical Notes: The limit is enforced in readback_part, content readers, and mime_subparts_read, starting from Curl_mime_read.

🤖 Was this summary useful? React with 👍 or 👎

Copy link
Copy Markdown

@augmentcode augmentcode Bot left a comment

Choose a reason for hiding this comment

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

Review completed. 1 suggestion posted.

Fix All in Augment

Comment augment review to trigger a new review at any time.

Comment thread lib/mime.c
@bagder bagder closed this in d087a7e Apr 20, 2026
@bagder bagder deleted the bagder/mime-nested-max branch April 20, 2026 12:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Development

Successfully merging this pull request may close these issues.

3 participants