openssh bugs - Apr 2023 - [Bug 3559] New: Mini memory leak and needless(?) const/static qualifier.

https://bugzilla.mindrot.org/show_bug.cgi?id=3559

            Bug ID: 3559
           Summary: Mini memory leak and needless(?) const/static
                    qualifier.
           Product: Portable OpenSSH
           Version: 8.5p1
          Hardware: Other
                OS: All
            Status: NEW
          Severity: enhancement
          Priority: P5
         Component: ssh
          Assignee: unassigned-bugs at mindrot.org
          Reporter: m.schmidt at emtec.com

in sshconnect2.c in function ssh_kex2() the function
kex_default_pk_alg() is called.

This function is from readconf.c and has the following prototype:
const char *kex_default_pk_alg(void);

The function looks like this:

const char *
kex_default_pk_alg(void)
{
    static char *pkalgs;

    if (pkalgs == NULL) {
        char *all_key;

        all_key = sshkey_alg_list(0, 0, 1, ',');
        pkalgs = match_filter_allowlist(KEX_DEFAULT_PK_ALG, all_key);
        free(all_key);
    }
    return pkalgs;
}


It internally buffers the result for match_filter_allowlist() in a
static variable, which makes it impossible to free the result and
essentially makes it leak.

Since the function is only called twice in the whole program (above
mentioned ssh_kex2() and dump_client_config()) and even only once in
each control paths, I believe the pkalgs could be made non-static and
could then be freed by the callers.

Also, both callers (ssh_kex2() and dump_client_config() compute
sshkey_alg_list(0, 0, 1, ','); prior to calling kex_default_pk_alg()
which then computes the same again, so that value could be passed to
kex_default_pk_alg() as a parameter).

If you are interested in fixing this I can make a proposed patch.

-- 
You are receiving this mail because:
You are watching the assignee of the bug.

https://bugzilla.mindrot.org/show_bug.cgi?id=3559

Markus Schmidt <m.schmidt at emtec.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Severity|enhancement                 |trivial

--- Comment #1 from Markus Schmidt <m.schmidt at emtec.com> ---
Changed to trivival

-- 
You are receiving this mail because:
You are watching the assignee of the bug.

https://bugzilla.mindrot.org/show_bug.cgi?id=3559

--- Comment #2 from Markus Schmidt <m.schmidt at emtec.com> ---

On further thought, the function could be abandoned alltogether and the
two callers could simply call 

 pkalgs = match_filter_allowlist(KEX_DEFAULT_PK_ALG, all_key);

themselves and free the result.

-- 
You are receiving this mail because:
You are watching the assignee of the bug.