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

Fix NULL pointer access #11024

Closed
wants to merge 1 commit into from
Closed

Fix NULL pointer access #11024

wants to merge 1 commit into from

Conversation

afalkenhahn
Copy link
Contributor

@afalkenhahn afalkenhahn commented Apr 25, 2023

We need to check against NULL here because kbd_callback can be called before ssh_attach stuffs the easy handle into the session's abstract storage.

We need to check against NULL here because it can be called before `ssh_attach` stuffs the easy handle into the session's abstract storage.
@jay
Copy link
Member

jay commented Apr 25, 2023

How come this has never been an issue before? Shouldn't data always be valid there? The only way I can see this happening is the "abstract" (user-supplied pointer) parameter was NULL the last time it was set, and AFAICT we only set abstract on init when CURL_DEBUG but I'm not sure why:

curl/lib/vssh/libssh2.c

Lines 3272 to 3278 in b16d1fa

#ifdef CURL_LIBSSH2_DEBUG
sshc->ssh_session = libssh2_session_init_ex(my_libssh2_malloc,
my_libssh2_free,
my_libssh2_realloc, data);
#else
sshc->ssh_session = libssh2_session_init();
#endif

Edit: Looks like 8b5f100 is biting us.

jay added a commit to jay/curl that referenced this pull request Apr 25, 2023
- Always set the libssh2 'abstract' user-pointer to the libcurl easy
  handle associated with the ssh session, so it is always passed to the
  ssh keyboard callback.

Prior to this change and since 8b5f100 (precedes curl 8.0.0), if libcurl
was built without CURL_DEBUG then it could crash during the ssh auth
phase due to a null dereference in the ssh keyboard callback.

Reported-by: Andreas Falkenhahn

Fixes curl#11024
Closes #xxxx
@jay
Copy link
Member

jay commented Apr 25, 2023

Closing in favor of #11026. @afalkenhahn please test

@jay jay closed this Apr 25, 2023
@afalkenhahn
Copy link
Contributor Author

I can confirm that #11026 fixes the issue.

jay added a commit that referenced this pull request Apr 26, 2023
- Always set the libssh2 'abstract' user-pointer to the libcurl easy
  handle associated with the ssh session, so it is always passed to the
  ssh keyboard callback.

Prior to this change and since 8b5f100 (precedes curl 8.0.0), if libcurl
was built without CURL_DEBUG then it could crash during the ssh auth
phase due to a null dereference in the ssh keyboard callback.

Reported-by: Andreas Falkenhahn

Fixes #11024
Closes #11026
@jay
Copy link
Member

jay commented Apr 26, 2023

Thanks

bch pushed a commit to bch/curl that referenced this pull request Jul 19, 2023
- Always set the libssh2 'abstract' user-pointer to the libcurl easy
  handle associated with the ssh session, so it is always passed to the
  ssh keyboard callback.

Prior to this change and since 8b5f100 (precedes curl 8.0.0), if libcurl
was built without CURL_DEBUG then it could crash during the ssh auth
phase due to a null dereference in the ssh keyboard callback.

Reported-by: Andreas Falkenhahn

Fixes curl#11024
Closes curl#11026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Development

Successfully merging this pull request may close these issues.

None yet

3 participants