Skip to content

Commit 5d414f1

Browse files
committed
Fix unresponsive cupsd process caused by a slow client
If client is very slow, it will slow cupsd process for other clients. The fix is the best effort without turning scheduler cupsd into multithreaded process which would be too complex and error-prone when backporting to 2.4.x series. The fix for unencrypted communication is to follow up on communication only if there is the whole line on input, and the waiting time is guarded by timeout. Encrypted communication now starts after we have the whole client hello packet, which conflicts with optional upgrade support to HTTPS via methods other than method OPTIONS, so this optional support defined in RFC 2817, section 3.1 is removed. Too slow or incomplete requests are handled by connection timeout. Fixes CVE-2025-58436
1 parent bed9c82 commit 5d414f1

6 files changed

Lines changed: 198 additions & 97 deletions

File tree

cups/http-private.h

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -121,6 +121,7 @@ extern "C" {
121121
* Constants...
122122
*/
123123

124+
# define _HTTP_MAX_BUFFER 32768 /* Size of read buffer */
124125
# define _HTTP_MAX_SBUFFER 65536 /* Size of (de)compression buffer */
125126
# define _HTTP_RESOLVE_DEFAULT 0 /* Just resolve with default options */
126127
# define _HTTP_RESOLVE_STDERR 1 /* Log resolve progress to stderr */
@@ -233,8 +234,8 @@ struct _http_s /**** HTTP connection structure ****/
233234
http_encoding_t data_encoding; /* Chunked or not */
234235
int _data_remaining;/* Number of bytes left (deprecated) */
235236
int used; /* Number of bytes used in buffer */
236-
char buffer[HTTP_MAX_BUFFER];
237-
/* Buffer for incoming data */
237+
char _buffer[HTTP_MAX_BUFFER];
238+
/* Old read buffer (deprecated) */
238239
int _auth_type; /* Authentication in use (deprecated) */
239240
unsigned char _md5_state[88]; /* MD5 state (deprecated) */
240241
char nonce[HTTP_MAX_VALUE];
@@ -308,6 +309,8 @@ struct _http_s /**** HTTP connection structure ****/
308309
/* Allocated field values */
309310
*default_fields[HTTP_FIELD_MAX];
310311
/* Default field values, if any */
312+
char buffer[_HTTP_MAX_BUFFER];
313+
/* Read buffer */
311314
};
312315
# endif /* !_HTTP_NO_PRIVATE */
313316

cups/http.c

Lines changed: 52 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,7 @@ static http_t *http_create(const char *host, int port,
5353
static void http_debug_hex(const char *prefix, const char *buffer,
5454
int bytes);
5555
#endif /* DEBUG */
56-
static ssize_t http_read(http_t *http, char *buffer, size_t length);
56+
static ssize_t http_read(http_t *http, char *buffer, size_t length, int timeout);
5757
static ssize_t http_read_buffered(http_t *http, char *buffer, size_t length);
5858
static ssize_t http_read_chunk(http_t *http, char *buffer, size_t length);
5959
static int http_send(http_t *http, http_state_t request,
@@ -1206,7 +1206,7 @@ httpGets(char *line, /* I - Line to read into */
12061206
return (NULL);
12071207
}
12081208

1209-
bytes = http_read(http, http->buffer + http->used, (size_t)(HTTP_MAX_BUFFER - http->used));
1209+
bytes = http_read(http, http->buffer + http->used, (size_t)(_HTTP_MAX_BUFFER - http->used), http->wait_value);
12101210

12111211
DEBUG_printf(("4httpGets: read " CUPS_LLFMT " bytes.", CUPS_LLCAST bytes));
12121212

@@ -1726,24 +1726,13 @@ httpPeek(http_t *http, /* I - HTTP connection */
17261726

17271727
ssize_t buflen; /* Length of read for buffer */
17281728

1729-
if (!http->blocking)
1730-
{
1731-
while (!httpWait(http, http->wait_value))
1732-
{
1733-
if (http->timeout_cb && (*http->timeout_cb)(http, http->timeout_data))
1734-
continue;
1735-
1736-
return (0);
1737-
}
1738-
}
1739-
17401729
if ((size_t)http->data_remaining > sizeof(http->buffer))
17411730
buflen = sizeof(http->buffer);
17421731
else
17431732
buflen = (ssize_t)http->data_remaining;
17441733

17451734
DEBUG_printf(("2httpPeek: Reading %d bytes into buffer.", (int)buflen));
1746-
bytes = http_read(http, http->buffer, (size_t)buflen);
1735+
bytes = http_read(http, http->buffer, (size_t)buflen, http->wait_value);
17471736

17481737
DEBUG_printf(("2httpPeek: Read " CUPS_LLFMT " bytes into buffer.",
17491738
CUPS_LLCAST bytes));
@@ -1764,9 +1753,9 @@ httpPeek(http_t *http, /* I - HTTP connection */
17641753
int zerr; /* Decompressor error */
17651754
z_stream stream; /* Copy of decompressor stream */
17661755

1767-
if (http->used > 0 && ((z_stream *)http->stream)->avail_in < HTTP_MAX_BUFFER)
1756+
if (http->used > 0 && ((z_stream *)http->stream)->avail_in < _HTTP_MAX_BUFFER)
17681757
{
1769-
size_t buflen = HTTP_MAX_BUFFER - ((z_stream *)http->stream)->avail_in;
1758+
size_t buflen = _HTTP_MAX_BUFFER - ((z_stream *)http->stream)->avail_in;
17701759
/* Number of bytes to copy */
17711760

17721761
if (((z_stream *)http->stream)->avail_in > 0 &&
@@ -2024,7 +2013,7 @@ httpRead2(http_t *http, /* I - HTTP connection */
20242013

20252014
if (bytes == 0)
20262015
{
2027-
ssize_t buflen = HTTP_MAX_BUFFER - (ssize_t)((z_stream *)http->stream)->avail_in;
2016+
ssize_t buflen = _HTTP_MAX_BUFFER - (ssize_t)((z_stream *)http->stream)->avail_in;
20282017
/* Additional bytes for buffer */
20292018

20302019
if (buflen > 0)
@@ -2774,20 +2763,54 @@ int /* O - 1 to continue, 0 to stop */
27742763
_httpUpdate(http_t *http, /* I - HTTP connection */
27752764
http_status_t *status) /* O - Current HTTP status */
27762765
{
2777-
char line[32768], /* Line from connection... */
2766+
char line[_HTTP_MAX_BUFFER], /* Line from connection... */
27782767
*value; /* Pointer to value on line */
27792768
http_field_t field; /* Field index */
27802769
int major, minor; /* HTTP version numbers */
27812770

27822771

27832772
DEBUG_printf(("_httpUpdate(http=%p, status=%p), state=%s", (void *)http, (void *)status, httpStateString(http->state)));
27842773

2774+
/* When doing non-blocking I/O, make sure we have a whole line... */
2775+
if (!http->blocking)
2776+
{
2777+
ssize_t bytes; /* Bytes "peeked" from connection */
2778+
2779+
/* See whether our read buffer is full... */
2780+
DEBUG_printf(("2_httpUpdate: used=%d", http->used));
2781+
2782+
if (http->used > 0 && !memchr(http->buffer, '\n', (size_t)http->used) && (size_t)http->used < sizeof(http->buffer))
2783+
{
2784+
/* No, try filling in more data... */
2785+
if ((bytes = http_read(http, http->buffer + http->used, sizeof(http->buffer) - (size_t)http->used, /*timeout*/0)) > 0)
2786+
{
2787+
DEBUG_printf(("2_httpUpdate: Read %d bytes.", (int)bytes));
2788+
http->used += (int)bytes;
2789+
}
2790+
}
2791+
2792+
/* Peek at the incoming data... */
2793+
if (!http->used || !memchr(http->buffer, '\n', (size_t)http->used))
2794+
{
2795+
/* Don't have a full line, tell the reader to try again when there is more data... */
2796+
DEBUG_puts("1_htttpUpdate: No newline in buffer yet.");
2797+
if ((size_t)http->used == sizeof(http->buffer))
2798+
*status = HTTP_STATUS_ERROR;
2799+
else
2800+
*status = HTTP_STATUS_CONTINUE;
2801+
return (0);
2802+
}
2803+
2804+
DEBUG_puts("2_httpUpdate: Found newline in buffer.");
2805+
}
2806+
27852807
/*
27862808
* Grab a single line from the connection...
27872809
*/
27882810

27892811
if (!httpGets(line, sizeof(line), http))
27902812
{
2813+
DEBUG_puts("1_httpUpdate: Error reading request line.");
27912814
*status = HTTP_STATUS_ERROR;
27922815
return (0);
27932816
}
@@ -4140,7 +4163,8 @@ http_debug_hex(const char *prefix, /* I - Prefix for line */
41404163
static ssize_t /* O - Number of bytes read or -1 on error */
41414164
http_read(http_t *http, /* I - HTTP connection */
41424165
char *buffer, /* I - Buffer */
4143-
size_t length) /* I - Maximum bytes to read */
4166+
size_t length, /* I - Maximum bytes to read */
4167+
int timeout) /* I - Wait timeout */
41444168
{
41454169
ssize_t bytes; /* Bytes read */
41464170

@@ -4149,7 +4173,7 @@ http_read(http_t *http, /* I - HTTP connection */
41494173

41504174
if (!http->blocking || http->timeout_value > 0.0)
41514175
{
4152-
while (!httpWait(http, http->wait_value))
4176+
while (!_httpWait(http, timeout, 1))
41534177
{
41544178
if (http->timeout_cb && (*http->timeout_cb)(http, http->timeout_data))
41554179
continue;
@@ -4252,7 +4276,7 @@ http_read_buffered(http_t *http, /* I - HTTP connection */
42524276
else
42534277
bytes = (ssize_t)length;
42544278

4255-
DEBUG_printf(("8http_read: Grabbing %d bytes from input buffer.",
4279+
DEBUG_printf(("8http_read_buffered: Grabbing %d bytes from input buffer.",
42564280
(int)bytes));
42574281

42584282
memcpy(buffer, http->buffer, (size_t)bytes);
@@ -4262,7 +4286,7 @@ http_read_buffered(http_t *http, /* I - HTTP connection */
42624286
memmove(http->buffer, http->buffer + bytes, (size_t)http->used);
42634287
}
42644288
else
4265-
bytes = http_read(http, buffer, length);
4289+
bytes = http_read(http, buffer, length, http->wait_value);
42664290

42674291
return (bytes);
42684292
}
@@ -4603,15 +4627,15 @@ http_set_timeout(int fd, /* I - File descriptor */
46034627
static void
46044628
http_set_wait(http_t *http) /* I - HTTP connection */
46054629
{
4606-
if (http->blocking)
4607-
{
4608-
http->wait_value = (int)(http->timeout_value * 1000);
4630+
http->wait_value = (int)(http->timeout_value * 1000);
46094631

4610-
if (http->wait_value <= 0)
4632+
if (http->wait_value <= 0)
4633+
{
4634+
if (http->blocking)
46114635
http->wait_value = 60000;
4636+
else
4637+
http->wait_value = 1000;
46124638
}
4613-
else
4614-
http->wait_value = 10000;
46154639
}
46164640

46174641

cups/tls-openssl.c

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -215,12 +215,14 @@ cupsMakeServerCredentials(
215215
// Save them...
216216
if ((bio = BIO_new_file(keyfile, "wb")) == NULL)
217217
{
218+
DEBUG_printf(("1cupsMakeServerCredentials: Unable to create private key file '%s': %s", keyfile, strerror(errno)));
218219
_cupsSetError(IPP_STATUS_ERROR_INTERNAL, strerror(errno), 0);
219220
goto done;
220221
}
221222

222223
if (!PEM_write_bio_PrivateKey(bio, pkey, NULL, NULL, 0, NULL, NULL))
223224
{
225+
DEBUG_puts("1cupsMakeServerCredentials: PEM_write_bio_PrivateKey failed.");
224226
_cupsSetError(IPP_STATUS_ERROR_INTERNAL, _("Unable to write private key."), 1);
225227
BIO_free(bio);
226228
goto done;
@@ -230,12 +232,14 @@ cupsMakeServerCredentials(
230232

231233
if ((bio = BIO_new_file(crtfile, "wb")) == NULL)
232234
{
235+
DEBUG_printf(("1cupsMakeServerCredentials: Unable to create certificate file '%s': %s", crtfile, strerror(errno)));
233236
_cupsSetError(IPP_STATUS_ERROR_INTERNAL, strerror(errno), 0);
234237
goto done;
235238
}
236239

237240
if (!PEM_write_bio_X509(bio, cert))
238241
{
242+
DEBUG_puts("1cupsMakeServerCredentials: PEM_write_bio_X509 failed.");
239243
_cupsSetError(IPP_STATUS_ERROR_INTERNAL, _("Unable to write X.509 certificate."), 1);
240244
BIO_free(bio);
241245
goto done;
@@ -1082,10 +1086,10 @@ _httpTLSStart(http_t *http) // I - Connection to server
10821086

10831087
if (!cupsMakeServerCredentials(tls_keypath, cn, 0, NULL, time(NULL) + 3650 * 86400))
10841088
{
1085-
DEBUG_puts("4_httpTLSStart: cupsMakeServerCredentials failed.");
1089+
DEBUG_printf(("4_httpTLSStart: cupsMakeServerCredentials failed: %s", cupsLastErrorString()));
10861090
http->error = errno = EINVAL;
10871091
http->status = HTTP_STATUS_ERROR;
1088-
_cupsSetError(IPP_STATUS_ERROR_INTERNAL, _("Unable to create server credentials."), 1);
1092+
// _cupsSetError(IPP_STATUS_ERROR_INTERNAL, _("Unable to create server credentials."), 1);
10891093
SSL_CTX_free(context);
10901094
_cupsMutexUnlock(&tls_mutex);
10911095

@@ -1346,14 +1350,17 @@ http_bio_read(BIO *h, // I - BIO data
13461350

13471351
http = (http_t *)BIO_get_data(h);
13481352

1349-
if (!http->blocking)
1353+
if (!http->blocking || http->timeout_value > 0.0)
13501354
{
13511355
/*
13521356
* Make sure we have data before we read...
13531357
*/
13541358

1355-
if (!_httpWait(http, 10000, 0))
1359+
while (!_httpWait(http, http->wait_value, 0))
13561360
{
1361+
if (http->timeout_cb && (*http->timeout_cb)(http, http->timeout_data))
1362+
continue;
1363+
13571364
#ifdef WIN32
13581365
http->error = WSAETIMEDOUT;
13591366
#else

0 commit comments

Comments
 (0)