Package home | Report new bug | New search | Development Roadmap Status: Open | Feedback | All | Closed Since Version 2.4.1

Bug #21229 fgets never times out with malicious server
Submitted: 2017-07-10 20:13 UTC
From: mmn Assigned: avb
Status: Closed Package: HTTP_Request2 (version 2.3.0)
PHP Version: Irrelevant OS: Debian
Roadmaps: (Not assigned)    
Subscription  


 [2017-07-10 20:13 UTC] mmn (Mikael Nordfeldth)
Description: ------------ The usage of 'fgets' in HTTP_Request2_SocketWrapper -> readLine makes it possible for remote servers to reply r-e-a-l-l-y s-l-o-w-l-y and thus blocking a PHP process. It seems that 'fread' will time out properly, while 'fgets' does not: https://secure.php.net/fgets vs. https://secure.php.net/fread In practice, fread seems to appropriately time out when reading from an extremely slow stream, while fgets simply continues until it reaches bufferSize (default 16Ki) or a newline (a malicious server can choose to never send a newline, causing a script to time out only after 16000 seconds because of the buffer size having to be filled up). It seems to fix this, we should have a small internal buffer and make sure to use 'fread' instead of 'fgets' but simulate the stop-at-newline functionality as it's often used to read in HTTP headers etc. A thread related to this in GNU social's use of HTTP_Request2 can be found here: https://git.gnu.io/gnu/gnu-social/issues/281#note_5674

Comments

 [2017-07-11 10:19 UTC] mmn (Mikael Nordfeldth)
 [2017-07-11 10:23 UTC] mmn (Mikael Nordfeldth)
I think the stream_select solution I have added as a patch here should work well as a solution too. It'll force fgets to stick within the given time parameter (unless both localTimeout and deadline is null, then null is given which is infinite time). I think I've formatted the patch properly for v2.3.0, let me know otherwise. I've patched this in the bundled library for GNU social, where I experienced this issue, here: https://git.gnu.io/gnu/gnu-social/commit/05a9c11c476b384e5ef3f3cc83b66406fcf7a378
 [2017-07-11 10:24 UTC] mmn (Mikael Nordfeldth)
Maybe the "reset timeout" code below this patch is no longer necessary, but I don't want to put too much into the patch either.
 [2020-07-26 17:06 UTC] avb (Alexey Borzov)
-Status: Open +Status: Closed -Assigned To: +Assigned To: avb
Fixed in Git: https://github.com/pear/HTTP_Request2/commit/da1c7675f4b7e1616fac6296391bc4736b3d7dba SocketWrapper uses only non-blocking sockets and relies on timeouts of stream_select()