From 19d9a655d0865bd58dd01ba8dd17ec93d054969a Mon Sep 17 00:00:00 2001 From: Oleksii Molchanov Date: Tue, 18 Aug 2026 00:40:21 +0300 Subject: [PATCH 1/2] Free the previous nonce before storing a nextnonce. auc_digest_info() stores the server-supplied Authentication-Info nextnonce over cda->cda_ac->ac_nonce without releasing what was there: n = auth_get_params(home, info->ai_params, "nextnonce=", &nextnonce, NULL); if (n <= 0) return n; cda->cda_ac->ac_nonce = nextnonce; auth_get_params() allocates the new string from `home`, and the pointer being overwritten is owned by that same home: it came either from the challenge via auth_digest_challenge_get(), or from an earlier Authentication-Info through this same assignment. The consequence is not "one leak per nonce" but "all but the last": the next challenge calls auth_digest_challenge_free_params(), which does release ac_nonce - so of N responses carrying nextnonce between two challenges, N-1 strings are lost. A server that sends Authentication-Info on every 2xx therefore leaks one nonce string per response for as long as the credentials stay valid. Like every other object in this family the block is allocated from a su_home and stays reachable through the home's block table, so no leak checker reports it; it shows up only as heap growth proportional to the response rate. The free follows the discipline the rest of the file already uses - auth_digest_challenge_free_params() at auth_digest.c:126, and the cnonce path in auc_digest_challenge(), both su_free() from ca_home before replacing. su_free() ignores NULL, so the first nextnonce on a fresh client is unaffected. --- libsofia-sip-ua/iptsec/auth_client.c | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/libsofia-sip-ua/iptsec/auth_client.c b/libsofia-sip-ua/iptsec/auth_client.c index f26f2b13..eca3f4a3 100644 --- a/libsofia-sip-ua/iptsec/auth_client.c +++ b/libsofia-sip-ua/iptsec/auth_client.c @@ -907,6 +907,13 @@ static int auc_digest_info(auth_client_t *ca, if (n <= 0) return n; + /* The nonce being replaced is owned by this home - it came either from the + * challenge, via auth_digest_challenge_get(), or from an earlier + * Authentication-Info through this same assignment. Without this free only + * the last one is ever released, by auth_digest_challenge_free_params() at + * the next challenge, so a server that sends nextnonce on every response + * loses one nonce string per response until then. su_free() ignores NULL. */ + su_free(home, (void *)cda->cda_ac->ac_nonce); cda->cda_ac->ac_nonce = nextnonce; return 1; From 2e7730e22504b8f1c8e19a8da7bc87eedf97ec43 Mon Sep 17 00:00:00 2001 From: Oleksii Molchanov Date: Tue, 18 Aug 2026 00:44:37 +0300 Subject: [PATCH 2/2] Add a test that a nextnonce does not strand the nonce it replaces. Covers the free added to auc_digest_info(). The test drives the public client API - auc_challenge(), auc_credentials(), auc_info() - and asserts on the authenticator's own allocation statistics rather than on any internal pointer, because what is being tested is a lifetime and not a value. One Authentication-Info is fed first, so that whatever the first update allocates is already accounted for; the assertion is then that a hundred further updates leave the number of live blocks unchanged. Measured on this test: with the fix live blocks 0 -> 0 bytes 15 -> 15 without it live blocks 1 -> 101 bytes 22 -> 2222 One block per update, and 2 200 bytes for a hundred of them - 22 bytes each, which is exactly the nonce string in the fixture plus its terminator. Three things about the test worth stating, because all three are easy to get wrong: The statistics have to be taken from the authenticator's own home, not from the home handed to auc_challenge(). ca_create() makes the authenticator a su_home_clone() of that home, and su_home_get_stats() reports only the home it is given - it does not aggregate clones, regardless of its include_clones argument. Reading the parent shows nothing whether the nonce is freed or not. With the fix in place both block counts are zero, so an assertion on their equality alone would also hold if the statistics were not being collected at all. The test therefore checks first that the instrument reads something (hsb_bytes > 0), and then that the surviving nonce is the one the server sent last - a free of the wrong pointer, or a missing assignment, would satisfy the count while losing the value. And a leak checker cannot see this at all: the strings are allocated from a su_home and stay reachable through the home's block table until the home is destroyed, so valgrind and the sanitizers report nothing. The home's own statistics are the only instrument that sees it. --- libsofia-sip-ua/iptsec/test_auth_digest.c | 123 ++++++++++++++++++++++ 1 file changed, 123 insertions(+) diff --git a/libsofia-sip-ua/iptsec/test_auth_digest.c b/libsofia-sip-ua/iptsec/test_auth_digest.c index 424fc175..dd7c5880 100644 --- a/libsofia-sip-ua/iptsec/test_auth_digest.c +++ b/libsofia-sip-ua/iptsec/test_auth_digest.c @@ -71,6 +71,8 @@ #include #include +#include +#include #include #include #include @@ -1299,6 +1301,126 @@ test_auth_client(void) END(); } +/* Test that a nextnonce in Authentication-Info replaces the stored nonce + without stranding the one it replaces. + + auc_digest_info() overwrites cda_ac->ac_nonce with the value it parses out of + Authentication-Info. Both the old and the new string are allocated from the + client's own home, and the old one is released by + auth_digest_challenge_free_params() only at the *next* challenge - so without + an explicit free there, a server that sends Authentication-Info on every 2xx + strands one nonce string per response for as long as the credentials last. + + Nothing leaks in the ordinary sense: the blocks stay reachable through the + home and are reclaimed when it is destroyed, so a leak checker sees nothing. + The home's own statistics do see it, which is what this test uses. */ +int test_auth_info_nextnonce(void) +{ + BEGIN(); + + { + char challenge[] = + PROTOCOL " 401 Unauthorized\r\n" + "Call-ID:0e3dc2b2-dcc6-1226-26ac-258b5ce429ab\r\n" + "CSeq:32439043 REGISTER\r\n" + "From:;tag=I8hFdg0H3OK\r\n" + "To:\r\n" + "Via:SIP/2.0/UDP 10.21.36.70:23800;branch=z9hG4bKJjKGu9vIHqf\r\n" + "WWW-Authenticate:Digest algorithm=MD5,nonce=\"nonce0\",realm=\"test-realm\"\r\n" + "Content-Length:0\r\n" + "\r\n"; + + char info[] = + PROTOCOL " 200 OK\r\n" + "Call-ID:0e3dc2b2-dcc6-1226-26ac-258b5ce429ab\r\n" + "CSeq:32439044 REGISTER\r\n" + "From:;tag=I8hFdg0H3OK\r\n" + "To:;tag=b\r\n" + "Via:SIP/2.0/UDP 10.21.36.70:23800;branch=z9hG4bKJjKGu9vIHqf\r\n" + "Authentication-Info:nextnonce=\"nonce-from-the-server\"\r\n" + "Content-Length:0\r\n" + "\r\n"; + + su_home_t *home; + msg_t *msg; + sip_t *sip; + auth_client_t *aucs = NULL; + su_home_stat_t hs0[1], hs1[1]; + int i; + + TEST_1(home = su_home_new(sizeof(*home))); + + TEST_1(msg = read_message(MSG_DO_EXTRACT_COPY, challenge)); + TEST_1(sip = sip_object(msg)); + TEST_1(sip->sip_www_authenticate); + TEST(auc_challenge(&aucs, home, sip->sip_www_authenticate, + sip_authorization_class), 1); + TEST_1(aucs != NULL); + msg_destroy(msg); + + TEST(auc_credentials(&aucs, home, "Digest:\"test-realm\":user:pass"), 1); + + /* The statistics have to come from the client's own home: auc_challenge() + makes the authenticator a *clone* of the home passed to it + (su_home_clone() in ca_create()), and su_home_get_stats() reports only + the home it is handed - it does not aggregate clones. Reading the parent + here would show nothing at all, whether the nonce is freed or not. */ + su_home_init_stats(aucs->ca_home); + + /* One update first, so that whatever the first one allocates is already + accounted for and the comparison below is about repetition alone. */ + TEST_1(msg = read_message(MSG_DO_EXTRACT_COPY, info)); + TEST_1(sip = sip_object(msg)); + TEST_1(sip->sip_authentication_info); + TEST(auc_info(&aucs, sip->sip_authentication_info, + sip_authorization_class), 1); + msg_destroy(msg); + + su_home_get_stats(aucs->ca_home, 0, hs0, sizeof hs0); + + for (i = 0; i < 100; i++) { + TEST_1(msg = read_message(MSG_DO_EXTRACT_COPY, info)); + TEST_1(sip = sip_object(msg)); + TEST_1(sip->sip_authentication_info); + TEST(auc_info(&aucs, sip->sip_authentication_info, + sip_authorization_class), 1); + msg_destroy(msg); + } + + su_home_get_stats(aucs->ca_home, 0, hs1, sizeof hs1); + + /* The assertion below compares two numbers that are both zero when the + fix is in place, so it would also hold if the statistics were not being + collected at all. Check first that the instrument reads something. */ + TEST_1(hs0->hs_blocks.hsb_bytes > 0); + + /* Each iteration replaces one string with another of the same length, so + the number of live blocks in the home must not move. Without the free in + auc_digest_info() it grows by exactly one per iteration. */ + TEST_1(hs1->hs_blocks.hsb_number == hs0->hs_blocks.hsb_number); + + /* And the nonce that survives has to be the one the server sent last: a + free of the wrong pointer, or a missing assignment, would satisfy the + block count above while losing the value. Taken after the statistics so + that building a header cannot disturb them. */ + { + msg_header_t *h = NULL; + char const *nonce; + + TEST(auc_authorization_headers(&aucs, home, "REGISTER", + (url_t *)"sip:ims3.so.noklab.net", + NULL, &h), 1); + TEST_1(h); + TEST_1(nonce = msg_header_find_param(h->sh_common, "nonce=")); + TEST_S(nonce, "\"nonce-from-the-server\""); + } + + su_home_unref(home); + } + + END(); +} + #if HAVE_FLOCK #include #endif @@ -1485,6 +1607,7 @@ int main(int argc, char *argv[]) retval |= test_digest(); retval |= test_digest_client(); retval |= test_auth_client(); + retval |= test_auth_info_nextnonce(); retval |= test_module_io(); su_deinit();