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

heap-buffer-overflow when using curl_multi_wait #7379

Closed
sylgal opened this issue Jul 12, 2021 · 2 comments
Closed

heap-buffer-overflow when using curl_multi_wait #7379

sylgal opened this issue Jul 12, 2021 · 2 comments
Labels

Comments

@sylgal
Copy link

@sylgal sylgal commented Jul 12, 2021

I did this

Using curl_multi_wait (400 connections), most of them cannot be established

I got the following crash:

==65100==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x61f0000ad2b0 at pc 0x0000004649d7 bp 0x00004e072680 sp 0x00004e071e28
READ of size 4 at 0x61f0000ad2b0 thread T11
    #0 0x4649d6 in poll (/tmp/a+0x4649d6)
    #1 0x40bb9292 in Curl_poll /usr/ports/ftp/curl/work/curl-7.77.0/lib/select.c:374:7
    #2 0x40baca06 in multi_wait /usr/ports/ftp/curl/work/curl-7.77.0/lib/multi.c:1269:14
    #3 0x40bac4da in curl_multi_wait /usr/ports/ftp/curl/work/curl-7.77.0/lib/multi.c:1398:10

0x61f0000ad2b0 is located 0 bytes to the right of 3120-byte region [0x61f0000ac680,0x61f0000ad2b0)
allocated by thread T11 here:
    #0 0x49147d in malloc (/tmp/a+0x49147d)
    #1 0x40bac73f in multi_wait /usr/ports/ftp/curl/work/curl-7.77.0/lib/multi.c:1158:12
    #2 0x40bac4da in curl_multi_wait /usr/ports/ftp/curl/work/curl-7.77.0/lib/multi.c:1398:10

I expected the following

No crash

curl/libcurl version

curl 7.77.0 (amd64-portbld-freebsd13.0) libcurl/7.77.0 OpenSSL/1.1.1k zlib/1.2.11 libssh2/1.9.0 nghttp2/1.43.0

operating system

FreeBSD fb13-x64-sds73 13.0-STABLE FreeBSD 13.0-STABLE #239 stable/13-n245926-68d6663afba-dirty: Thu Jul 8 01:28:22 UTC 2021 root@fb13-x64-test:/usr/obj/usr/src/amd64.amd64/sys/SOLIDSERVER amd64

No crash after applying the following patch:

--- lib/multi.c.orig    2021-07-02 09:50:52.535241000 +0000
+++ lib/multi.c 2021-07-02 09:53:18.924601000 +0000
@@ -1176,7 +1176,7 @@
 #ifdef USE_WINSOCK
         long mask = 0;
 #endif
-        if(bitmap & GETSOCK_READSOCK(i)) {
+        if((bitmap & GETSOCK_READSOCK(i)) && VALID_SOCK((sockbunch[i]))) {
           s = sockbunch[i];
 #ifdef USE_WINSOCK
           mask |= FD_READ|FD_ACCEPT|FD_CLOSE;
@@ -1185,7 +1185,7 @@
           ufds[nfds].events = POLLIN;
           ++nfds;
         }
-        if(bitmap & GETSOCK_WRITESOCK(i)) {
+        if((bitmap & GETSOCK_WRITESOCK(i)) && VALID_SOCK((sockbunch[i]))) {
           s = sockbunch[i];
 #ifdef USE_WINSOCK
           mask |= FD_WRITE|FD_CONNECT|FD_CLOSE;
@piru
Copy link

@piru piru commented Jul 12, 2021

It appears that 51c0ebc added this.

@danielgustafsson
Copy link
Member

@danielgustafsson danielgustafsson commented Jul 12, 2021

@sylgal off the cuff this seems sane, can you please open a PR with your patch to give it some CI exposure?

@jay jay added the crash label Jul 13, 2021
@jay jay closed this in 6a66f72 Jul 16, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
4 participants