I can confirm that the checks for null data below prevent the crashes that I
observed when client drives SSL sockets and steps on read/write errors.
Should I create a PR with these changes?
---
lib/cf-socket.c | 16 ++++++++++------
1 file changed, 10 insertions(+), 6 deletions(-)
diff --git a/lib/cf-socket.c b/lib/cf-socket.c
index e42b4a87b..98bca9f17 100644
--- a/lib/cf-socket.c
+++ b/lib/cf-socket.c
@@ -863,9 +863,11 @@ static ssize_t nw_in_read(void *reader_ctx,
else {
char buffer[STRERROR_LEN];
- failf(rctx->data, "Recv failure: %s",
- Curl_strerror(sockerr, buffer, sizeof(buffer)));
- rctx->data->state.os_errno = sockerr;
+ if(rctx->data) {
+ failf(rctx->data, "Recv failure: %s",
+ Curl_strerror(sockerr, buffer, sizeof(buffer)));
+ rctx->data->state.os_errno = sockerr;
+ }
*err = CURLE_RECV_ERROR;
nread = -1;
}
@@ -1329,9 +1331,11 @@ static ssize_t cf_socket_send(struct Curl_cfilter *cf,
struct Curl_easy *data,
}
else {
char buffer[STRERROR_LEN];
- failf(data, "Send failure: %s",
- Curl_strerror(sockerr, buffer, sizeof(buffer)));
- data->state.os_errno = sockerr;
+ if(data) {
+ failf(data, "Send failure: %s",
+ Curl_strerror(sockerr, buffer, sizeof(buffer)));
+ data->state.os_errno = sockerr;
+ }
*err = CURLE_SEND_ERROR;
}
}
--
-----Original Message-----
From: curl-library <curl-library-boun...@lists.haxx.se> On Behalf Of Dmitry
Karpov via curl-library
Sent: Tuesday, January 16, 2024 1:25 PM
To: libcurl development <curl-library@lists.haxx.se>
Cc: Dmitry Karpov <dkar...@roku.com>
Subject: [EXTERNAL] RE: Crash in nw_in_read when client drives SSL sockets
The same protection should be probably done in the cf_socket_send() as well.
-----Original Message-----
From: curl-library <curl-library-boun...@lists.haxx.se> On Behalf Of Dmitry
Karpov via curl-library
Sent: Tuesday, January 16, 2024 11:53 AM
To: libcurl development <curl-library@lists.haxx.se>
Cc: Dmitry Karpov <dkar...@roku.com>
Subject: [EXTERNAL] Crash in nw_in_read when client drives SSL sockets
Hi All,
Migrating from 7.84 to 8.5.0, I stepped on some crash regression in some "3rd
party" client code, which drives SSL sockets after using libcurl transfer for
protocol "upgrade" purposes.
It looks some like some custom WebSocket implementation, which client code uses
for different HTTP clients.
The problem happens when something gets wrong with the connection (maybe it is
suddenly closed)
The call stack looks like following:
Program terminated with signal SIGSEGV, Segmentation fault.
#0 0xb647c6ce in Curl_failf (data=0x0, fmt=0xb64a70c8 = "Recv failure: %s") at
../../../../../../os/dist/curl/Carbon14_dev.OBJ/src/lib/curl_trc.c:90
#1 0xb6473e78 in nw_in_read (reader_ctx=0xb43fc928, buf=<optimized out>, len=5,
err=0xb43fc964) at
../../../../../../os/dist/curl/Carbon14_dev.OBJ/src/lib/cf-socket.c:866
#2 0xb6475304 in cf_socket_recv (cf=0xb4d10810, data=0x0, buf=0xb4dd6ddb,
len=5, err=0xb43fc964) at
../../../../../../os/dist/curl/Carbon14_dev.OBJ/src/lib/cf-socket.c:1414
#3 0xb6498234 in ossl_bio_cf_in_read (bio=0xb4d10560, buf=0xb4dd6ddb, blen=5)
at ../../../../../../os/dist/curl/Carbon14_dev.OBJ/src/lib/vtls/openssl.c:763
#4 0xb6240c9c in bread_conv (bio=<optimized out>, data=<optimized out>,
datal=<optimized out>, readbytes=0xb43fc9cc) at crypto/bio/bio_meth.c:123
#5 0xb6242bd2 in bio_read_intern (b=0xb4d10560, data=0xb4dd6ddb, dlen=5,
readbytes=0xb43fc9cc) at crypto/bio/bio_lib.c:292
#6 0xb6242c70 in BIO_read (dlen=<optimized out>, data=<optimized out>,
b=<optimized out>) at crypto/bio/bio_lib.c:318
#7 0xb6242c70 in BIO_read (b=<optimized out>, data=<optimized out>,
dlen=<optimized out>) at crypto/bio/bio_lib.c:310
#8 0xb6428ff4 in ssl3_read_n (s=0xb4d527a0, n=5, max=<optimized out>,
extend=<optimized out>, clearold=1, readbytes=0xb43fca4c) at
ssl/record/rec_layer_s3.c:293
#9 0xb64299ba in ssl3_get_record (s=0xb4d527a0) at ssl/record/ssl3_record.c:210
#10 0xb64299ba in ssl3_read_bytes (s=0xb4d527a0, type=<optimized out>,
recvd_type=<optimized out>, buf=<optimized out>, len=<optimized out>,
peek=<optimized out>, readbytes=<optimized out>) at
ssl/record/rec_layer_s3.c:1350
#11 0xb64153ea in ssl3_read_internal (s=0xb4d527a0, buf=0xb43fcb68, len=8192,
peek=0, readbytes=0xb43fcb1c) at ssl/s3_lib.c:4463
#12 0xb641542e in ssl3_read (s=<optimized out>, buf=<optimized out>,
len=<optimized out>, readbytes=<optimized out>) at ssl/s3_lib.c:4486
#13 0xb6418ba2 in SSL_read (s=<optimized out>, buf=<optimized out>,
num=<optimized out>) at ssl/ssl_lib.c:1958 ...
It seems that the problem happens when the Curl transfer which originated the
WebSocket was suddenly completed (i.e. server closed the connection), and
client maybe does its last read to detect that, and it expects to get -1 from
read.
The nw_in_read() function has the error reporting part:
failf(rctx->data, "Recv failure: %s",
Curl_strerror(sockerr, buffer, sizeof(buffer)));
rctx->data->state.os_errno = sockerr;
I think it should be wrapped into if(rctx->data) {} block to avoid crashes when
nw_in_read() function is called when some transfer is completed, and the easy
handle is null in the socket filters.
Thanks,
Dmitry Karpov
--
Unsubscribe: https://lists.haxx.se/mailman/listinfo/curl-library
Etiquette: https://curl.se/mail/etiquette.html
--
Unsubscribe: https://lists.haxx.se/mailman/listinfo/curl-library
Etiquette: https://curl.se/mail/etiquette.html
--
Unsubscribe: https://lists.haxx.se/mailman/listinfo/curl-library
Etiquette: https://curl.se/mail/etiquette.html
