Skip to content

Fix: memory leak in auth_create_digest_http_message() in lib/vauth/digest.c#20862

Closed
huanghuihui0904 wants to merge 1 commit intocurl:masterfrom
huanghuihui0904:fix-memory-leak-auth_create_digest_http_message
Closed

Fix: memory leak in auth_create_digest_http_message() in lib/vauth/digest.c#20862
huanghuihui0904 wants to merge 1 commit intocurl:masterfrom
huanghuihui0904:fix-memory-leak-auth_create_digest_http_message

Conversation

@huanghuihui0904
Copy link
Copy Markdown
Contributor

@huanghuihui0904 huanghuihui0904 commented Mar 9, 2026

Bug description

While reviewing auth_create_digest_http_message() in lib/vauth/digest.c, I noticed an error path that may leak the heap-allocated buffer hashthis.

hashthis is allocated here (line 800):

hashthis = curl_maprintf("%s:%s", request, uripath);

In the auth-int branch, if the subsequent hash() call fails, execution jumps to oom: without freeing hashthis (line 806–813):

if(digest->qop && curl_strequal(digest->qop, "auth-int")) {
    /* We do not support auth-int for PUT or POST */
    char hashed[65];
    char *hashthis2;

    result = hash(hashbuf, (const unsigned char *)"", 0);
    if(result)
      goto oom;
}

However, the cleanup block does not free hashthis (line 953–960):

oom:
    curlx_free(nonce_quoted);
    curlx_free(realm_quoted);
    curlx_free(uri_quoted);
    curlx_free(userp_quoted);
    if(result)
        curlx_dyn_free(&response);
    return result;

As a result, if the hash() call fails in the auth-int branch after hashthis has been allocated, the function jumps to oom: and returns without freeing hashthis, which may lead to a memory leak.

Fix

Free hashthis before jumping to oom: when hash() fails. The fix is included in the commit.

@bagder
Copy link
Copy Markdown
Member

bagder commented Mar 9, 2026

This causes double-free of that pointer in the other branches now.

@testclutch
Copy link
Copy Markdown

Analysis of PR #20862 at 84f0c771:

Test ../../tests/http/test_17_ssl_use.py::TestSSLUse::test_17_09_ssl_min_max[TLSv1.3-none-none] failed, which has NOT been flaky recently, so there could be a real issue in this PR. Note that this test has failed in 2 different CI jobs (the link just goes to one of them).

Test ../../tests/http/test_17_ssl_use.py::TestSSLUse::test_17_09_ssl_min_max[TLSv1.3-none-1.2] failed, which has NOT been flaky recently, so there could be a real issue in this PR. Note that this test has failed in 2 different CI jobs (the link just goes to one of them).

Test ../../tests/http/test_17_ssl_use.py::TestSSLUse::test_17_09_ssl_min_max[TLSv1.3-none-1.3] failed, which has NOT been flaky recently, so there could be a real issue in this PR. Note that this test has failed in 2 different CI jobs (the link just goes to one of them).

Test ../../tests/http/test_17_ssl_use.py::TestSSLUse::test_17_09_ssl_min_max[TLSv1.3-tlsv1-none] failed, which has NOT been flaky recently, so there could be a real issue in this PR. Note that this test has failed in 2 different CI jobs (the link just goes to one of them).

Test ../../tests/http/test_17_ssl_use.py::TestSSLUse::test_17_09_ssl_min_max[TLSv1.3-tlsv1-1.2] failed, which has NOT been flaky recently, so there could be a real issue in this PR. Note that this test has failed in 2 different CI jobs (the link just goes to one of them).

Test ../../tests/http/test_17_ssl_use.py::TestSSLUse::test_17_09_ssl_min_max[TLSv1.3-tlsv1-1.3] failed, which has NOT been flaky recently, so there could be a real issue in this PR. Note that this test has failed in 2 different CI jobs (the link just goes to one of them).

Test ../../tests/http/test_17_ssl_use.py::TestSSLUse::test_17_09_ssl_min_max[TLSv1.3-tlsv1.0-none] failed, which has NOT been flaky recently, so there could be a real issue in this PR. Note that this test has failed in 2 different CI jobs (the link just goes to one of them).

There are more failures, but that's enough from Gha.

Generated by Testclutch

…gest.c

Signed-off-by: huanghuihui0904 <625173@qq.com>
@huanghuihui0904 huanghuihui0904 force-pushed the fix-memory-leak-auth_create_digest_http_message branch from 84f0c77 to c569449 Compare March 9, 2026 08:02
@huanghuihui0904
Copy link
Copy Markdown
Contributor Author

This causes double-free of that pointer in the other branches now.

Good catch, thanks! I've updated the fix to avoid that and pushed a new commit.

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 potential memory leak in auth_create_digest_http_message() (lib/vauth/digest.c) when the auth-int branch hits an early error and jumps to oom:.

Changes:

  • Free hashthis before goto oom when hash() fails in the auth-int path.

💡 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 closed this in cbb5544 Mar 9, 2026
@bagder
Copy link
Copy Markdown
Member

bagder commented Mar 9, 2026

Thanks!

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.

4 participants