Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Rectify heap corruption due to an uninitialized pointer #2925

Closed
wants to merge 1 commit into from

Conversation

nnposter
Copy link

@nnposter nnposter commented Aug 31, 2024

The PR addresses two closely related issues in publickey_canauth() in nse_libssh2:

  • An uninitialized pointer was getting freed up, resulting in memory corruption. Specifically, callback function
    static int publickey_canauth_cb (LIBSSH2_SESSION *session, unsigned char **sig,
    is expected to allocate memory on the heap and pass its address back via previously uninitialized pointer *sig. This memory is then getting copied elsewhere and automatically freed up by libssh2:
    LIBSSH2_FREE(session, sig);
    The callback function was leaving this pointer in its original uninitialized state. The change is that the function now always errors out, which removes the authentication signature data from the execution path.
  • The logic for determining whether a particular combination of a username and a public key was accepted by the target was deficient in that it broadly treated certain errors as a match indication, leading to false positives when the passed public key was corrupted:
    if (rc == LIBSSH2_ERROR_ALLOC || rc == LIBSSH2_ERROR_PUBLICKEY_UNVERIFIED)
    The change is that the match is determined by a specific error originating from inside the callback function.

The PR is a more comprehensive version of Julijan Nedic's #2924 and together with PR #2919 represents a fix for #2917.

The PR will be committed after September 6, 2024, unless concerns are raised.

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

Successfully merging this pull request may close these issues.

1 participant