Skip to content

tool_urlglob: fix memory-leak on glob range overflow#20956

Closed
bagder wants to merge 1 commit intomasterfrom
bagder/urlglob-leak
Closed

tool_urlglob: fix memory-leak on glob range overflow#20956
bagder wants to merge 1 commit intomasterfrom
bagder/urlglob-leak

Conversation

@bagder
Copy link
Copy Markdown
Member

@bagder bagder commented Mar 17, 2026

Follow-up to 5f273fd

Pointed out by Codex Security

Follow-up to 5f273fd

Pointed out by Codex Security
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

Fixes a memory leak in the URL globbing parser when a {...} set grows beyond the allowed size (“range overflow”), ensuring allocated set elements are freed on error.

Changes:

  • Replace a direct early return on set-size overflow in glob_set() with setting an error code and jumping to the shared cleanup path (goto error).

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

You can also share your feedback on Copilot code review. Take the survey.

@bagder bagder marked this pull request as ready for review March 17, 2026 12:41
@vszakats
Copy link
Copy Markdown
Member

vszakats commented Mar 17, 2026

Reminds me of gcc analyzer finding a leak here in #20921:

src/tool_urlglob.c:48:17: warning: leak of 'malloc(8)' [CWE-401] [-Wanalyzer-malloc-leak]

will be interesting to retest after merging, and perhaps unsilence if fixed.

vszakats pushed a commit to vszakats/curl that referenced this pull request Mar 17, 2026
Follow-up to 5f273fd

Pointed out by Codex Security

Closes curl#20956
@bagder bagder closed this in 1098e10 Mar 17, 2026
@bagder bagder deleted the bagder/urlglob-leak branch March 17, 2026 13:55
@vszakats
Copy link
Copy Markdown
Member

Reminds me of gcc analyzer finding a leak here in #20921:

src/tool_urlglob.c:48:17: warning: leak of 'malloc(8)' [CWE-401] [-Wanalyzer-malloc-leak]

will be interesting to retest after merging, and perhaps unsilence if fixed.

It's a different leak (if real.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Development

Successfully merging this pull request may close these issues.

3 participants