[BUG] OTP R12B-3: HTTP "space after headers" bug in inet_drv.c

[Edwin Fine]

First of all, many thanks to Claes Wikstrom for isolating and finding the module in which this bug lives (and for doing it so quickly). I originally thought this was a Yaws bug, and emailed Claes with the symptoms. It's quite repeatable (test code at end of email). He traced the bug to inet_drv.c and emailed me back with a small test program and details, and I offered to report the bug. Then I got curious as to what it could have been, so I did some debugging.

The bug is in inet_drv.c. It will cause the end of the HTTP headers to be missed if the first character following the last CRLF (the one on its "own line") is a space or tab. This causes Yaws to time out after 30 seconds waiting for more header data that will never arrive. I am sure this will be true for httpd, too.

The culprit (well, sort of; if you read on you will see why) is the following code on line 8324:

if (SP(ptr2+1)) {
    ptr1 = ptr2+1;
    len = n - plen;
}
else
    goto done;

Consider the following HTTP POST data:

"POST /invalid/url HTTP/1.1\r\n"
"Connection: close\r\n"
"Host: localhost:8000\r\n"
"User-Agent: perl post\r\n"
"Content-Length: 4\r\n"
"Content-Type: text/xml; charset=utf-8\r\n"
"\r\n"
" postdata..." <-- Note this data starts with a space character

When the inet_drv.c code gets to the last CRLF, it doesn't detect that it's the last CRLF (end of headers) and checks to see if the next character is a space or tab using the macro SP(ptr2+1). If there's any data following the last CRLF and it starts with a space or tab, the code as it stands now will think it's another header line and try to get more data, then time out when nothing arrives.

What needed to be done was to check to see if the data (in that particular state) was an LF or CRLF standing on its own, and only check for a space following that if not. This check correctly detects the CRLF that terminates the header data. I added two lines of code to inet_drv.c to fix that error, and the Erlang test code (kindly supplied by Claes, and at the end of this email) now runs without error. I can't guarantee that the fix will work under all circumstances, and it needs to be tested thoroughly, but it's a starting point and illustrates the problem. (I would say that the HTTP header parsing in inet_drv.c probably could be beefed up a little).

The patch to inet_drv.c is as follows (tabs may not be correct because I reformatted the code):

8318a8319,8323
>
> /* Test needed in case buffer is in form "\r\n\srequestdata" where \s is SP or TAB */
> if (((plen == 1) && NL(ptr)) || ((plen == 2) && CRNL(ptr)))
>     goto done;
>

Claes's test code is here (slightly modified by me to make it easier to test the normal case and the "bug" case). You can run it as post:p(Host,Port,data) or post:p(Host,Port,bug).

Regards,
Edwin Fine
======================================
-module(post).
-compile(export_all).

p() ->
    p({127,0,0,1},8000,data).

p(Host, Port, What) when What =:= bug; What =:= data ->
   {ok, S} = gen_tcp:connect(Host, Port, [{active, true}]),
   gen_tcp:send(S, select_data(What)),
   recloop().

recloop() ->
   receive
       X ->
           io:format("GOT ~p~n", [X]),
           recloop()
   after 4000 ->
           timeout
   end.

select_data(What) ->
    case What of
        bug ->
            bug();
        data ->
            data()
    end.

data() ->
   Msg = "dfoo",
   H = "POST /invalid/url HTTP/1.1\r\n"
       "Connection: close\r\n"
       "Host: localhost:8000\r\n"
       "User-Agent: perl post\r\n"
       "Content-Length: 4\r\n"
       "Content-Type: text/xml; charset=utf-8\r\n"
       "\r\n",
   H ++ Msg.


%% space bug
bug() ->
   Msg = " foo",
   H = "POST /invalid/url HTTP/1.1\r\n"
       "Connection: close\r\n"
       "Host: localhost:8000\r\n"
       "User-Agent: perl post\r\n"
       "Content-Length: 4\r\n"
       "Content-Type: text/xml; charset=utf-8\r\n"
       "\r\n",
   H ++ Msg.



--
The great enemy of the truth is very often not the lie -- deliberate, contrived and dishonest, but the myth, persistent, persuasive, and unrealistic. Belief in myths allows the comfort of opinion without the discomfort of thought.
John F. Kennedy 35th president of US 1961-1963 (1917 - 1963)
_______________________________________________
erlang-bugs mailing list
erlang-bugs@...
http://www.erlang.org/mailman/listinfo/erlang-bugs

[Sverker Eriksson]

Thanks, for the bug-report.

The following simple patch seems to do the trick:

-if (SP(ptr2+1)) {
+if (SP(ptr2+1) && plen>2) {
    ptr1 = ptr2+1;
    len = n - plen;
}
else
    goto done;



Will be included in next release.

/Sverker, Erlang/OTP Ericsson


Edwin Fine wrote:

> First of all, many thanks to Claes Wikstrom for isolating and finding the
> module in which this bug lives (and for doing it so quickly). I originally
> thought this was a Yaws bug, and emailed Claes with the symptoms. It's quite
> repeatable (test code at end of email). He traced the bug to inet_drv.c and
> emailed me back with a small test program and details, and I offered to
> report the bug. Then I got curious as to what it could have been, so I did
> some debugging.
>
> The bug is in inet_drv.c. It will cause the end of the HTTP headers to be
> missed if the first character following the last CRLF (the one on its "own
> line") is a space or tab. This causes Yaws to time out after 30 seconds
> waiting for more header data that will never arrive. I am sure this will be
> true for httpd, too.
>
> The culprit (well, sort of; if you read on you will see why) is the
> following code on line 8324:
>
> *if (SP(ptr2+1)) {*
>     ptr1 = ptr2+1;
>     len = n - plen;
> }
> else
>     goto done;
>
> Consider the following HTTP POST data:
>
> "POST /invalid/url HTTP/1.1\r\n"
> "Connection: close\r\n"
> "Host: localhost:8000\r\n"
> "User-Agent: perl post\r\n"
> "Content-Length: 4\r\n"
> "Content-Type: text/xml; charset=utf-8\r\n"
> "\r\n"
> " postdata..." <-- Note this data starts with a space character
>
> When the inet_drv.c code gets to the last CRLF, it doesn't detect that it's
> the last CRLF (end of headers) and checks to see if the next character is a
> space or tab using the macro SP(ptr2+1). If there's any data following the
> last CRLF and it starts with a space or tab, the code as it stands now will
> think it's another header line and try to get more data, then time out when
> nothing arrives.
>
> What needed to be done was to check to see if the data (in that particular
> state) was an LF or CRLF standing on its own, and only check for a space
> following that if not. This check correctly detects the CRLF that terminates
> the header data. I added two lines of code to inet_drv.c to fix that error,
> and the Erlang test code (kindly supplied by Claes, and at the end of this
> email) now runs without error. I can't guarantee that the fix will work
> under all circumstances, and it needs to be tested thoroughly, but it's a
> starting point and illustrates the problem. (I would say that the HTTP
> header parsing in inet_drv.c probably could be beefed up a little).
>
> The patch to inet_drv.c is as follows (tabs may not be correct because I
> reformatted the code):
>
> 8318a8319,8323
>  
>> /* Test needed in case buffer is in form "\r\n\srequestdata" where \s is
>>    
> SP or TAB */
>  
>> if (((plen == 1) && NL(ptr)) || ((plen == 2) && CRNL(ptr)))
>>     goto done;
>>
>>    
>
> Claes's test code is here (slightly modified by me to make it easier to test
> the normal case and the "bug" case). You can run it as
> post:p(Host,Port,data) or post:p(Host,Port,bug).
>
> Regards,
> Edwin Fine
> ======================================
> -module(post).
> -compile(export_all).
>
> p() ->
>     p({127,0,0,1},8000,data).
>
> p(Host, Port, What) when What =:= bug; What =:= data ->
>    {ok, S} = gen_tcp:connect(Host, Port, [{active, true}]),
>    gen_tcp:send(S, select_data(What)),
>    recloop().
>
> recloop() ->
>    receive
>        X ->
>            io:format("GOT ~p~n", [X]),
>            recloop()
>    after 4000 ->
>            timeout
>    end.
>
> select_data(What) ->
>     case What of
>         bug ->
>             bug();
>         data ->
>             data()
>     end.
>
> data() ->
>    Msg = "dfoo",
>    H = "POST /invalid/url HTTP/1.1\r\n"
>        "Connection: close\r\n"
>        "Host: localhost:8000\r\n"
>        "User-Agent: perl post\r\n"
>        "Content-Length: 4\r\n"
>        "Content-Type: text/xml; charset=utf-8\r\n"
>        "\r\n",
>    H ++ Msg.
>
>
> %% space bug
> bug() ->
>    Msg = " foo",
>    H = "POST /invalid/url HTTP/1.1\r\n"
>        "Connection: close\r\n"
>        "Host: localhost:8000\r\n"
>        "User-Agent: perl post\r\n"
>        "Content-Length: 4\r\n"
>        "Content-Type: text/xml; charset=utf-8\r\n"
>        "\r\n",
>    H ++ Msg.
>
>
>
>  
> ------------------------------------------------------------------------
>
> _______________________________________________
> erlang-bugs mailing list
> erlang-bugs@...
> http://www.erlang.org/mailman/listinfo/erlang-bugs

_______________________________________________
erlang-bugs mailing list
erlang-bugs@...
http://www.erlang.org/mailman/listinfo/erlang-bugs