From 0499655ba4448fd2ae232d82bd78b47809e56591 Mon Sep 17 00:00:00 2001 From: Marc Alexander Date: Sat, 21 Jun 2014 11:55:54 +0200 Subject: [PATCH 1/5] [ticket/12755] Add timeout to remote upload to prevent infinite loop PHPBB3-12755 --- phpBB/includes/functions_upload.php | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/phpBB/includes/functions_upload.php b/phpBB/includes/functions_upload.php index 73ac1df2d2..afffff1351 100644 --- a/phpBB/includes/functions_upload.php +++ b/phpBB/includes/functions_upload.php @@ -466,6 +466,9 @@ class fileupload var $max_height = 0; var $error_prefix = ''; + /** @var int Timeout for remote upload */ + var $upload_timeout = 5; + /** * Init file upload class. * @@ -785,6 +788,9 @@ class fileupload return $file; } + // Set a proper timeout for the socket + socket_set_timeout($fsock, $this->upload_timeout); + // Make sure $path not beginning with / if (strpos($path, '/') === 0) { @@ -797,6 +803,8 @@ class fileupload $get_info = false; $data = ''; + $upload_start = time(); + while (!@feof($fsock)) { if ($get_info) @@ -813,6 +821,13 @@ class fileupload } $data .= $block; + + // Cancel upload if we exceed timeout + if ((time() - $upload_start) >= $this->upload_timeout) + { + $file = new fileerror($user->lang[$this->error_prefix . 'EMPTY_REMOTE_DATA']); + return $file; + } } else { From 8817b5937747f0b82e180bd3ce7d38b8aa68577e Mon Sep 17 00:00:00 2001 From: Marc Alexander Date: Mon, 23 Jun 2014 20:35:36 +0200 Subject: [PATCH 2/5] [ticket/12755] Add language string for timed out remote upload PHPBB3-12755 --- phpBB/includes/functions_upload.php | 2 +- phpBB/language/en/common.php | 1 + phpBB/language/en/posting.php | 1 + 3 files changed, 3 insertions(+), 1 deletion(-) diff --git a/phpBB/includes/functions_upload.php b/phpBB/includes/functions_upload.php index afffff1351..f4b9262d19 100644 --- a/phpBB/includes/functions_upload.php +++ b/phpBB/includes/functions_upload.php @@ -825,7 +825,7 @@ class fileupload // Cancel upload if we exceed timeout if ((time() - $upload_start) >= $this->upload_timeout) { - $file = new fileerror($user->lang[$this->error_prefix . 'EMPTY_REMOTE_DATA']); + $file = new fileerror($user->lang[$this->error_prefix . 'REMOTE_UPLOAD_TIMEOUT']); return $file; } } diff --git a/phpBB/language/en/common.php b/phpBB/language/en/common.php index cc38804fe2..2d3710e15c 100644 --- a/phpBB/language/en/common.php +++ b/phpBB/language/en/common.php @@ -88,6 +88,7 @@ $lang = array_merge($lang, array( 'AVATAR_PARTIAL_UPLOAD' => 'The specified file was only partially uploaded.', 'AVATAR_PHP_SIZE_NA' => 'The avatar’s filesize is too large.
The maximum allowed filesize set in php.ini could not be determined.', 'AVATAR_PHP_SIZE_OVERRUN' => 'The avatar’s filesize is too large. The maximum allowed upload size is %1$d %2$s.
Please note this is set in php.ini and cannot be overridden.', + 'AVATAR_REMOTE_UPLOAD_TIMEOUT' => 'The specified avatar could not be uploaded because the request timed out.', 'AVATAR_URL_INVALID' => 'The URL you specified is invalid.', 'AVATAR_URL_NOT_FOUND' => 'The file specified could not be found.', 'AVATAR_WRONG_FILESIZE' => 'The avatar’s filesize must be between 0 and %1$d %2$s.', diff --git a/phpBB/language/en/posting.php b/phpBB/language/en/posting.php index df411c3228..5316011f4e 100644 --- a/phpBB/language/en/posting.php +++ b/phpBB/language/en/posting.php @@ -178,6 +178,7 @@ $lang = array_merge($lang, array( 'QUOTE_DEPTH_EXCEEDED' => 'You may embed only %1$d quotes within each other.', + 'REMOTE_UPLOAD_TIMEOUT' => 'The specified file could not be uploaded because the request timed out.', 'SAVE' => 'Save', 'SAVE_DATE' => 'Saved at', 'SAVE_DRAFT' => 'Save draft', From 5ee1e07e1731cfa58e815c4a805fb188b0986640 Mon Sep 17 00:00:00 2001 From: Marc Alexander Date: Tue, 24 Jun 2014 11:53:32 +0200 Subject: [PATCH 3/5] [ticket/12755] Change upload in remote_upload() method to fit get_remote_file PHPBB3-12755 --- phpBB/includes/functions_upload.php | 26 ++++++++++++++------------ 1 file changed, 14 insertions(+), 12 deletions(-) diff --git a/phpBB/includes/functions_upload.php b/phpBB/includes/functions_upload.php index f4b9262d19..c6e2dddf3d 100644 --- a/phpBB/includes/functions_upload.php +++ b/phpBB/includes/functions_upload.php @@ -467,7 +467,7 @@ class fileupload var $error_prefix = ''; /** @var int Timeout for remote upload */ - var $upload_timeout = 5; + var $upload_timeout = 6; /** * Init file upload class. @@ -788,9 +788,6 @@ class fileupload return $file; } - // Set a proper timeout for the socket - socket_set_timeout($fsock, $this->upload_timeout); - // Make sure $path not beginning with / if (strpos($path, '/') === 0) { @@ -801,9 +798,12 @@ class fileupload fputs($fsock, "HOST: " . $host . "\r\n"); fputs($fsock, "Connection: close\r\n\r\n"); + // Set a proper timeout for the socket + socket_set_timeout($fsock, $this->upload_timeout); + $get_info = false; $data = ''; - $upload_start = time(); + $timer_stop = time() + $this->upload_timeout; while (!@feof($fsock)) { @@ -821,13 +821,6 @@ class fileupload } $data .= $block; - - // Cancel upload if we exceed timeout - if ((time() - $upload_start) >= $this->upload_timeout) - { - $file = new fileerror($user->lang[$this->error_prefix . 'REMOTE_UPLOAD_TIMEOUT']); - return $file; - } } else { @@ -862,6 +855,15 @@ class fileupload } } } + + $stream_meta_data = stream_get_meta_data($fsock); + + // Cancel upload if we exceed timeout + if (!empty($stream_meta_data['timed_out']) || time() >= $timer_stop) + { + $file = new fileerror($user->lang[$this->error_prefix . 'REMOTE_UPLOAD_TIMEOUT']); + return $file; + } } @fclose($fsock); From 309dbb4ef9d6b2c29aa6294002ce1a7d4da2b099 Mon Sep 17 00:00:00 2001 From: Marc Alexander Date: Tue, 24 Jun 2014 19:07:49 +0200 Subject: [PATCH 4/5] [ticket/12755] Terminate upload loop if upload reaches filesize Terminate the upload loop if the expected filesize has been reached instead of trying to read more bytes until the timeout has been reached. PHPBB3-12755 --- phpBB/includes/functions_upload.php | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/phpBB/includes/functions_upload.php b/phpBB/includes/functions_upload.php index c6e2dddf3d..daa3550205 100644 --- a/phpBB/includes/functions_upload.php +++ b/phpBB/includes/functions_upload.php @@ -803,13 +803,23 @@ class fileupload $get_info = false; $data = ''; + $length = false; $timer_stop = time() + $this->upload_timeout; - while (!@feof($fsock)) + while (!($length && $filesize >= $length) && !@feof($fsock)) { if ($get_info) { - $block = @fread($fsock, 1024); + if ($length) + { + // Don't attempt to read past end of file if server indicated length + $block = @fread($fsock, min($length - $filesize, 1024)); + } + else + { + $block = @fread($fsock, 1024); + } + $filesize += strlen($block); if ($remote_max_filesize && $filesize > $remote_max_filesize) From 8b3cc9a6c494ecf7ec3262925b9e0c1381c0154e Mon Sep 17 00:00:00 2001 From: Marc Alexander Date: Tue, 24 Jun 2014 19:53:32 +0200 Subject: [PATCH 5/5] [ticket/12755] Apply de morgan to conditional PHPBB3-12755 --- phpBB/includes/functions_upload.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/phpBB/includes/functions_upload.php b/phpBB/includes/functions_upload.php index daa3550205..69f10911ec 100644 --- a/phpBB/includes/functions_upload.php +++ b/phpBB/includes/functions_upload.php @@ -806,7 +806,7 @@ class fileupload $length = false; $timer_stop = time() + $this->upload_timeout; - while (!($length && $filesize >= $length) && !@feof($fsock)) + while ((!$length || $filesize < $length) && !@feof($fsock)) { if ($get_info) {