Fri Sep 2 19:57:00 CEST 2011
a request, that's the application's prerogative. I understand that in
the CGI case, you want to prevent something like that from happening,
but that's beyond Warp's purview. The question is can we expose enough
primitives to make it possible for you to implement this at the
So from that perspective, I'm not sure if 96311d would really be
considered a vulnerability in Warp. Isn't your patch making it
impossible for an application to run in unbounded time? It might make
more sense to add a specific timeout each time the CGI app is called
(possibly via timeout) to ensure it responds appropriately. But if
I'm not mistaken, this isn't even necessary in the CGI case, as the
Response value will be returned by your code and will not be affected
by the response time of the CGI app itself.
Before we make more commits, let's make sure we're on the same page
about what needs to be done, and then make the fewest possible changes
to Warp in order to achieve our goals.
On Wed, Oct 12, 2011 at 9:08 AM, Kazu Yamamoto <kazu at iij.ad.jp> wrote:
> I confirmed that the following patch fixes this vulnerability. Just
> =A0 =A0 =A0 =A0https://github.com/kazu-yamamoto/wai/commit/96311d8040b649=
>> Hello Michael,
>> Sorry for this late response.
>> I noticed that if an exception handler is set in a Haskell thread, it
>> works even if the thread is killed. So, the settingsPauseForApp
>> approach is enough to me. Michael has alread reverted it but I want it
>> Before that, I would like to make sure two things:
>> 1) Would you take a look at "cgiApp'" defined in the following?
>> =A0 =A0 =A0 https://github.com/kazu-yamamoto/wai-app-file-cgi/blob/maste=
>> =A0 =A0Since a sub-process is created, there are two iteratees: The
>> =A0 =A0original iteratee consumes HTTP request body and passes it to CGI=
>> =A0 =A0Another iteratee to consumes output from CGI is returned as
>> =A0 =A0ResponseEnumerator.
>> =A0 =A0Are the error handlings in "cgiApp'" reasonable from Michael's
>> =A0 =A0point of view?
>> 2) I noticed that the commit of settingsPauseForApp does not work.
>> =A0 =A0The timer is paused anyway in serveConnection. Suppose that a nas=
>> =A0 =A0client specifies Content-Length:, for exapmle, to 10 bytes and
>> =A0 =A0sends only 5 bytes only and stops. Since the timer is paused and
>> =A0 =A0there is no chance for an iteratee to resume the timer, the
>> =A0 =A0connection is not closed by time out.
>> =A0 =A0I'm sure that this happens in the case of ResponseEnumerator. I
>> =A0 =A0suspect this happens in the case of ResponseFile and
>> =A0 =A0ResponseBuilder, too. In other words, a bad guy can make massive
>> =A0 =A0connections to Warp, which will not be closed by time out.
>> =A0 =A0I guess pausing in serveConnection is not a good idea.
>>  https://github.com/yesodweb/wai/commit/d2b6c66abef939bb1396d576e7541=
>>> On Thu, Oct 6, 2011 at 9:34 AM, Kazu Yamamoto <kazu at iij.ad.jp> wrote:
>>>>>> Mighttpd executes a sub process and creates a pair of pipes for
>>>>>> CGI. If timeout happens, it seems to me that there is no way to kill
>>>>>> the sub process and close the pipes with this scheme.
>>>>>> I would like to register a house-keeping action to Wrap's timer.
>>>>> So it sounds like instead of the solution we just put in, we should
>>>>> just expose the ability to use Warp's timeout code directly. This
>>>>> shouldn't be a problem:
>>>>> * Expose the Timeout module (maybe in its own package, could be usefu=
>>>>> to others)
>>>>> * Add an extra settingsTimeoutManager :: IO Manager. That way you can
>>>>> create the manager in Mighttpd and then reuse it in Warp.
>>>>> Would this address the issue?
>>>> I think so.
>>> OK, that one's even easier to implement. Please check out the most
>>> recent commit. I also realized that the Warp module already exports
>>> all the functions (I think) you need to use the timeout module; let me
>>> know if something's missing.
>> web-devel mailing list
>> web-devel at haskell.org
> web-devel mailing list
> web-devel at haskell.org
More information about the web-devel