Skip to content

Commit 40008d7

Browse files
michaelrsweetzdohnal
authored andcommitted
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 ffbb0de commit 40008d7

7 files changed

Lines changed: 181 additions & 98 deletions

File tree

cups/http-private.h

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -83,6 +83,7 @@ extern "C" {
8383
// Constants...
8484
//
8585

86+
# define _HTTP_MAX_BUFFER 32768 /* Size of read buffer */
8687
# define _HTTP_MAX_SBUFFER 65536 /* Size of (de)compression buffer */
8788

8889
# define _HTTP_TLS_NONE 0 /* No TLS options */
@@ -157,8 +158,8 @@ struct _http_s // HTTP connection structure
157158
http_encoding_t data_encoding; /* Chunked or not */
158159
int _data_remaining;/* Number of bytes left (deprecated) */
159160
int used; /* Number of bytes used in buffer */
160-
char buffer[HTTP_MAX_BUFFER];
161-
/* Buffer for incoming data */
161+
char _buffer[HTTP_MAX_BUFFER];
162+
/* Old read buffer (deprecated) */
162163
int _auth_type; /* Authentication in use (deprecated) */
163164
unsigned char _md5_state[88]; /* MD5 state (deprecated) */
164165
char nonce[HTTP_MAX_VALUE];
@@ -231,6 +232,8 @@ struct _http_s // HTTP connection structure
231232
*default_fields[HTTP_FIELD_MAX];
232233
/* Default field values, if any */
233234
/**** New in CUPS 2.5 ****/
235+
char buffer[_HTTP_MAX_BUFFER];
236+
/* Read buffer */
234237
char qop[HTTP_MAX_VALUE];
235238
/* Quality of Protection (qop) value from WWW-Authenticate */
236239
};

cups/http.c

Lines changed: 52 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@ static http_t *http_create(const char *host, int port, http_addrlist_t *addrlis
3737
#ifdef DEBUG
3838
static void http_debug_hex(const char *prefix, const char *buffer, int bytes);
3939
#endif // DEBUG
40-
static ssize_t http_read(http_t *http, char *buffer, size_t length);
40+
static ssize_t http_read(http_t *http, char *buffer, size_t length, int timeout);
4141
static ssize_t http_read_buffered(http_t *http, char *buffer, size_t length);
4242
static ssize_t http_read_chunk(http_t *http, char *buffer, size_t length);
4343
static bool http_send(http_t *http, http_state_t request, const char *uri);
@@ -1435,7 +1435,7 @@ httpGets2(http_t *http, // I - HTTP connection
14351435
return (NULL);
14361436
}
14371437

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

14401440
DEBUG_printf("4httpGets2: read " CUPS_LLFMT " bytes.", CUPS_LLCAST bytes);
14411441

@@ -1919,24 +1919,13 @@ httpPeek(http_t *http, // I - HTTP connection
19191919
// Buffer small reads for better performance...
19201920
ssize_t buflen; // Length of read for buffer
19211921

1922-
if (!http->blocking)
1923-
{
1924-
while (!httpWait(http, http->wait_value))
1925-
{
1926-
if (http->timeout_cb && (*http->timeout_cb)(http, http->timeout_data))
1927-
continue;
1928-
1929-
return (0);
1930-
}
1931-
}
1932-
19331922
if ((size_t)http->data_remaining > sizeof(http->buffer))
19341923
buflen = sizeof(http->buffer);
19351924
else
19361925
buflen = (ssize_t)http->data_remaining;
19371926

19381927
DEBUG_printf("2httpPeek: Reading %d bytes into buffer.", (int)buflen);
1939-
bytes = http_read(http, http->buffer, (size_t)buflen);
1928+
bytes = http_read(http, http->buffer, (size_t)buflen, http->wait_value);
19401929

19411930
DEBUG_printf("2httpPeek: Read " CUPS_LLFMT " bytes into buffer.", CUPS_LLCAST bytes);
19421931
if (bytes > 0)
@@ -1954,9 +1943,9 @@ httpPeek(http_t *http, // I - HTTP connection
19541943
int zerr; // Decompressor error
19551944
z_stream stream; // Copy of decompressor stream
19561945

1957-
if (http->used > 0 && ((z_stream *)http->stream)->avail_in < HTTP_MAX_BUFFER)
1946+
if (http->used > 0 && ((z_stream *)http->stream)->avail_in < _HTTP_MAX_BUFFER)
19581947
{
1959-
size_t buflen = HTTP_MAX_BUFFER - ((z_stream *)http->stream)->avail_in;
1948+
size_t buflen = _HTTP_MAX_BUFFER - ((z_stream *)http->stream)->avail_in;
19601949
// Number of bytes to copy
19611950

19621951
if (((z_stream *)http->stream)->avail_in > 0 && ((z_stream *)http->stream)->next_in > http->sbuffer)
@@ -2205,7 +2194,7 @@ httpRead2(http_t *http, // I - HTTP connection
22052194

22062195
if (bytes == 0)
22072196
{
2208-
ssize_t buflen = HTTP_MAX_BUFFER - (ssize_t)((z_stream *)http->stream)->avail_in;
2197+
ssize_t buflen = _HTTP_MAX_BUFFER - (ssize_t)((z_stream *)http->stream)->avail_in;
22092198
// Additional bytes for buffer
22102199

22112200
if (buflen > 0)
@@ -2904,17 +2893,51 @@ int // O - 1 to continue, 0 to stop
29042893
_httpUpdate(http_t *http, // I - HTTP connection
29052894
http_status_t *status) // O - Current HTTP status
29062895
{
2907-
char line[32768], // Line from connection...
2896+
char line[_HTTP_MAX_BUFFER], // Line from connection...
29082897
*value; // Pointer to value on line
29092898
http_field_t field; // Field index
29102899
int major, minor; // HTTP version numbers
29112900

29122901

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

2904+
// When doing non-blocking I/O, make sure we have a whole line...
2905+
if (!http->blocking)
2906+
{
2907+
ssize_t bytes; // Bytes "peeked" from connection
2908+
2909+
// See whether our read buffer is full...
2910+
DEBUG_printf("2_httpUpdate: used=%d", http->used);
2911+
2912+
if (http->used > 0 && !memchr(http->buffer, '\n', (size_t)http->used) && (size_t)http->used < sizeof(http->buffer))
2913+
{
2914+
// No, try filling in more data...
2915+
if ((bytes = http_read(http, http->buffer + http->used, sizeof(http->buffer) - (size_t)http->used, /*timeout*/0)) > 0)
2916+
{
2917+
DEBUG_printf("2_httpUpdate: Read %d bytes.", (int)bytes);
2918+
http->used += (int)bytes;
2919+
}
2920+
}
2921+
2922+
// Peek at the incoming data...
2923+
if (!http->used || !memchr(http->buffer, '\n', (size_t)http->used))
2924+
{
2925+
// Don't have a full line, tell the reader to try again when there is more data...
2926+
DEBUG_puts("1_htttpUpdate: No newline in buffer yet.");
2927+
if ((size_t)http->used == sizeof(http->buffer))
2928+
*status = HTTP_STATUS_ERROR;
2929+
else
2930+
*status = HTTP_STATUS_CONTINUE;
2931+
return (0);
2932+
}
2933+
2934+
DEBUG_puts("2_httpUpdate: Found newline in buffer.");
2935+
}
2936+
29152937
// Grab a single line from the connection...
29162938
if (!httpGets2(http, line, sizeof(line)))
29172939
{
2940+
DEBUG_puts("1_httpUpdate: Error reading request line.");
29182941
*status = HTTP_STATUS_ERROR;
29192942
return (0);
29202943
}
@@ -4100,7 +4123,8 @@ http_debug_hex(const char *prefix, // I - Prefix for line
41004123
static ssize_t // O - Number of bytes read or -1 on error
41014124
http_read(http_t *http, // I - HTTP connection
41024125
char *buffer, // I - Buffer
4103-
size_t length) // I - Maximum bytes to read
4126+
size_t length, // I - Maximum bytes to read
4127+
int timeout) // I - Wait timeout
41044128
{
41054129
ssize_t bytes; // Bytes read
41064130

@@ -4109,7 +4133,7 @@ http_read(http_t *http, // I - HTTP connection
41094133

41104134
if (!http->blocking || http->timeout_value > 0.0)
41114135
{
4112-
while (!httpWait(http, http->wait_value))
4136+
while (!_httpWait(http, timeout, 1))
41134137
{
41144138
if (http->timeout_cb && (*http->timeout_cb)(http, http->timeout_data))
41154139
continue;
@@ -4212,7 +4236,7 @@ http_read_buffered(http_t *http, // I - HTTP connection
42124236
else
42134237
bytes = (ssize_t)length;
42144238

4215-
DEBUG_printf("8http_read: Grabbing %d bytes from input buffer.", (int)bytes);
4239+
DEBUG_printf("8http_read_buffered: Grabbing %d bytes from input buffer.", (int)bytes);
42164240

42174241
memcpy(buffer, http->buffer, (size_t)bytes);
42184242
http->used -= (int)bytes;
@@ -4222,7 +4246,7 @@ http_read_buffered(http_t *http, // I - HTTP connection
42224246
}
42234247
else
42244248
{
4225-
bytes = http_read(http, buffer, length);
4249+
bytes = http_read(http, buffer, length, http->wait_value);
42264250
}
42274251

42284252
return (bytes);
@@ -4535,16 +4559,14 @@ http_set_timeout(int fd, // I - File descriptor
45354559
static void
45364560
http_set_wait(http_t *http) // I - HTTP connection
45374561
{
4538-
if (http->blocking)
4539-
{
4540-
http->wait_value = (int)(http->timeout_value * 1000);
4562+
http->wait_value = (int)(http->timeout_value * 1000);
45414563

4542-
if (http->wait_value <= 0)
4543-
http->wait_value = 60000;
4544-
}
4545-
else
4564+
if (http->wait_value <= 0)
45464565
{
4547-
http->wait_value = 10000;
4566+
if (http->blocking)
4567+
http->wait_value = 60000;
4568+
else
4569+
http->wait_value = 1000;
45484570
}
45494571
}
45504572

cups/tls-gnutls.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1832,7 +1832,7 @@ _httpTLSStart(http_t *http) // I - Connection to server
18321832

18331833
if (!cupsCreateCredentials(tls_keypath, false, CUPS_CREDPURPOSE_SERVER_AUTH, CUPS_CREDTYPE_DEFAULT, CUPS_CREDUSAGE_DEFAULT_TLS, NULL, NULL, NULL, NULL, NULL, cn, /*email*/NULL, 0, NULL, NULL, time(NULL) + 3650 * 86400))
18341834
{
1835-
DEBUG_puts("4_httpTLSStart: cupsCreateCredentials failed.");
1835+
DEBUG_printf("4_httpTLSStart: cupsCreateCredentials failed: %s", cupsGetErrorString());
18361836
http->error = errno = EINVAL;
18371837
http->status = HTTP_STATUS_ERROR;
18381838
cupsMutexUnlock(&tls_mutex);

cups/tls-openssl.c

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1900,7 +1900,7 @@ _httpTLSStart(http_t *http) // I - Connection to server
19001900

19011901
if (!cupsCreateCredentials(tls_keypath, false, CUPS_CREDPURPOSE_SERVER_AUTH, CUPS_CREDTYPE_DEFAULT, CUPS_CREDUSAGE_DEFAULT_TLS, NULL, NULL, NULL, NULL, NULL, cn, NULL, 0, NULL, NULL, time(NULL) + 3650 * 86400))
19021902
{
1903-
DEBUG_puts("4_httpTLSStart: cupsCreateCredentials failed.");
1903+
DEBUG_printf("4_httpTLSStart: cupsCreateCredentials failed: %s", cupsGetErrorString());
19041904
http->error = errno = EINVAL;
19051905
http->status = HTTP_STATUS_ERROR;
19061906
SSL_CTX_free(context);
@@ -2190,11 +2190,14 @@ http_bio_read(BIO *h, // I - BIO data
21902190
http = (http_t *)BIO_get_data(h);
21912191
DEBUG_printf("9http_bio_read: http=%p", (void *)http);
21922192

2193-
if (!http->blocking)
2193+
if (!http->blocking || http->timeout_value > 0.0)
21942194
{
21952195
// Make sure we have data before we read...
2196-
if (!_httpWait(http, 10000, 0))
2196+
while (!_httpWait(http, http->wait_value, 0))
21972197
{
2198+
if (http->timeout_cb && (*http->timeout_cb)(http, http->timeout_data))
2199+
continue;
2200+
21982201
#ifdef WIN32
21992202
http->error = WSAETIMEDOUT;
22002203
#else

0 commit comments

Comments
 (0)