From 03f880f24d1e05f86fd32a0b550b40579cf24085 Mon Sep 17 00:00:00 2001 From: David Carlier Date: Thu, 22 Aug 2024 22:41:39 +0100 Subject: [PATCH 1/4] Fixed GH-15547: curl_multi_wait expects a signed int for timeout. confusion might come from the previous argument type. PHP expects ms so we check it fits integer boundaries before the cast. raising a warning at least for stable branches. --- ext/curl/multi.c | 7 ++++++- ext/curl/tests/gh15547.phpt | 19 +++++++++++++++++++ 2 files changed, 25 insertions(+), 1 deletion(-) create mode 100644 ext/curl/tests/gh15547.phpt diff --git a/ext/curl/multi.c b/ext/curl/multi.c index e8c32301d2e4d..96ad62775b7c4 100644 --- a/ext/curl/multi.c +++ b/ext/curl/multi.c @@ -187,7 +187,12 @@ PHP_FUNCTION(curl_multi_select) mh = Z_CURL_MULTI_P(z_mh); - error = curl_multi_wait(mh->multi, NULL, 0, (unsigned long) (timeout * 1000.0), &numfds); + if (!(timeout >= ((double)INT_MIN / 1000.0) && timeout <= ((double)INT_MAX / 1000.0))) { + php_error_docref(NULL, E_WARNING, "timeout must be between %d and %d", (INT_MIN / 1000), (INT_MAX / 1000)); + RETURN_LONG(-1); + } + + error = curl_multi_wait(mh->multi, NULL, 0, (int) (timeout * 1000.0), &numfds); if (CURLM_OK != error) { SAVE_CURLM_ERROR(mh, error); RETURN_LONG(-1); diff --git a/ext/curl/tests/gh15547.phpt b/ext/curl/tests/gh15547.phpt new file mode 100644 index 0000000000000..ee315268e2a6f --- /dev/null +++ b/ext/curl/tests/gh15547.phpt @@ -0,0 +1,19 @@ +--TEST-- +GH-15547 - overflow on timeout argument +--EXTENSIONS-- +curl +--FILE-- + +--EXPECTF-- +Warning: curl_multi_select(): timeout must be between %s and %s in %s on line %d +int(-1) + +Warning: curl_multi_select(): timeout must be between %s and %s in %s on line %d +int(-1) +int(0) From e8e7fcd0239584b9cbdad931fb70904ed006b619 Mon Sep 17 00:00:00 2001 From: David Carlier Date: Fri, 23 Aug 2024 06:19:43 +0100 Subject: [PATCH 2/4] changes from feedback --- ext/curl/multi.c | 4 ++-- ext/curl/tests/gh15547.phpt | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/ext/curl/multi.c b/ext/curl/multi.c index 96ad62775b7c4..f8c59a2e0b020 100644 --- a/ext/curl/multi.c +++ b/ext/curl/multi.c @@ -187,8 +187,8 @@ PHP_FUNCTION(curl_multi_select) mh = Z_CURL_MULTI_P(z_mh); - if (!(timeout >= ((double)INT_MIN / 1000.0) && timeout <= ((double)INT_MAX / 1000.0))) { - php_error_docref(NULL, E_WARNING, "timeout must be between %d and %d", (INT_MIN / 1000), (INT_MAX / 1000)); + if (!(timeout >= 0.0 && timeout <= ((double)INT_MAX / 1000.0))) { + php_error_docref(NULL, E_WARNING, "timeout must be between 0 and %d", (int)ceilf((double)INT_MAX / 1000)); RETURN_LONG(-1); } diff --git a/ext/curl/tests/gh15547.phpt b/ext/curl/tests/gh15547.phpt index ee315268e2a6f..ab1c16ecc578c 100644 --- a/ext/curl/tests/gh15547.phpt +++ b/ext/curl/tests/gh15547.phpt @@ -11,9 +11,9 @@ var_dump(curl_multi_select($mh, 2500000)); var_dump(curl_multi_select($mh, 1000000)); ?> --EXPECTF-- -Warning: curl_multi_select(): timeout must be between %s and %s in %s on line %d +Warning: curl_multi_select(): timeout must be between 0 and %d in %s on line %d int(-1) -Warning: curl_multi_select(): timeout must be between %s and %s in %s on line %d +Warning: curl_multi_select(): timeout must be between 0 and %d in %s on line %d int(-1) int(0) From 5ecb5d9f608d7f233db440de2433efddd5085f39 Mon Sep 17 00:00:00 2001 From: David Carlier Date: Fri, 23 Aug 2024 11:32:02 +0100 Subject: [PATCH 3/4] setting curl multi error when it overflows --- ext/curl/multi.c | 1 + ext/curl/tests/gh15547.phpt | 12 +++++++++++- 2 files changed, 12 insertions(+), 1 deletion(-) diff --git a/ext/curl/multi.c b/ext/curl/multi.c index f8c59a2e0b020..873ee5bfdd6b5 100644 --- a/ext/curl/multi.c +++ b/ext/curl/multi.c @@ -189,6 +189,7 @@ PHP_FUNCTION(curl_multi_select) if (!(timeout >= 0.0 && timeout <= ((double)INT_MAX / 1000.0))) { php_error_docref(NULL, E_WARNING, "timeout must be between 0 and %d", (int)ceilf((double)INT_MAX / 1000)); + SAVE_CURLM_ERROR(mh, CURLM_BAD_FUNCTION_ARGUMENT); RETURN_LONG(-1); } diff --git a/ext/curl/tests/gh15547.phpt b/ext/curl/tests/gh15547.phpt index ab1c16ecc578c..7cdb714cd4c66 100644 --- a/ext/curl/tests/gh15547.phpt +++ b/ext/curl/tests/gh15547.phpt @@ -1,5 +1,5 @@ --TEST-- -GH-15547 - overflow on timeout argument +GH-15547 - curl_multi_select overflow on timeout argument --EXTENSIONS-- curl --FILE-- @@ -7,13 +7,23 @@ curl $mh = curl_multi_init(); var_dump(curl_multi_select($mh, -2500000)); +var_dump(curl_multi_strerror(curl_multi_errno($mh))); +curl_multi_close($mh); +$mh = curl_multi_init(); var_dump(curl_multi_select($mh, 2500000)); +var_dump(curl_multi_strerror(curl_multi_errno($mh))); +curl_multi_close($mh); +$mh = curl_multi_init(); var_dump(curl_multi_select($mh, 1000000)); +var_dump(curl_multi_strerror(curl_multi_errno($mh))); ?> --EXPECTF-- Warning: curl_multi_select(): timeout must be between 0 and %d in %s on line %d int(-1) +string(43) "A libcurl function was given a bad argument" Warning: curl_multi_select(): timeout must be between 0 and %d in %s on line %d int(-1) +string(43) "A libcurl function was given a bad argument" int(0) +string(8) "No error" From f6e6c712c384b6e94215ff4abe3d10be59fef5ac Mon Sep 17 00:00:00 2001 From: David Carlier Date: Fri, 23 Aug 2024 11:54:09 +0100 Subject: [PATCH 4/4] fixing missing enum case --- ext/curl/multi.c | 2 ++ ext/curl/tests/gh15547.phpt | 4 ++-- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/ext/curl/multi.c b/ext/curl/multi.c index 873ee5bfdd6b5..70cc7e0366410 100644 --- a/ext/curl/multi.c +++ b/ext/curl/multi.c @@ -189,7 +189,9 @@ PHP_FUNCTION(curl_multi_select) if (!(timeout >= 0.0 && timeout <= ((double)INT_MAX / 1000.0))) { php_error_docref(NULL, E_WARNING, "timeout must be between 0 and %d", (int)ceilf((double)INT_MAX / 1000)); +#ifdef CURLM_BAD_FUNCTION_ARGUMENT SAVE_CURLM_ERROR(mh, CURLM_BAD_FUNCTION_ARGUMENT); +#endif RETURN_LONG(-1); } diff --git a/ext/curl/tests/gh15547.phpt b/ext/curl/tests/gh15547.phpt index 7cdb714cd4c66..bbb1d5c5b0365 100644 --- a/ext/curl/tests/gh15547.phpt +++ b/ext/curl/tests/gh15547.phpt @@ -20,10 +20,10 @@ var_dump(curl_multi_strerror(curl_multi_errno($mh))); --EXPECTF-- Warning: curl_multi_select(): timeout must be between 0 and %d in %s on line %d int(-1) -string(43) "A libcurl function was given a bad argument" +%s Warning: curl_multi_select(): timeout must be between 0 and %d in %s on line %d int(-1) -string(43) "A libcurl function was given a bad argument" +%s int(0) string(8) "No error"