Hacker News Reader

Comment by zwp | Hacker News Reader

Small potatoes but this:

        for(char * i = this->URL; *i; ++ i)
        {
            if(i[0] == '.' && i[1] == '.' && (i[2] == '/' || i[2] == '\\'))

misses the case where the double dot is not followed by a slash (eg http://example.com/foo/..). Currently you're almost certainly okay (read(2) will _generally_ (OpenGroup says "implementation specific") return EISDIR) but beware if you should implement a mod_autoindex-alike.

ps. Somebody else said it already but: nice read :)

View on HN

Bigger potatoes: output sanitization?

Eg only whitespace is trimmed from cookies, then in webserver.Outgoing.cc:

    Socket << "\r\nSet-Cookie: " << Current->Key << "=" << Current->Value;

Lacewing splits input lines on "\r\n" but browsers often happily accept "\n" as a line delimiter (see 19.3 of RFC 2616 for Postel-esque tolerance discussion). So the client can send a request header like:

  Cookie: foo=bar\n<naughty stuff>\r\n

to manipulate the server response, cf "response splitting attack".

This is not discussed in the 37 pages(!) of the new cookie RFC 6265 :-/

It doesn't automatically send back any cookies the client sends, only the ones that have been modified by the application code (eg. via Request.Cookie("name", "new value")).

Making the parser split input lines on both "\r\n" and "\n" is a good idea. Thanks!

Done that, it's on github.

Cool.

> only the ones that have been modified by the application code

Sounds good (after a quick skim again I still can't see that but I'll take your word for it!).

I've had two attempts to build and test but unfortunately failed: currently can't build (src/unix/Client.cc:242: error: cannot convert ‘sockaddr_in’ to ‘const sockaddr’) and hello_world.c dumped core on me this time yesterday (Ubuntu 10.10).

"Work in progress" as you say... but it may just scratch an itch. I will come back again in a couple of months :) Thanks.

Someone actually just submitted a pull request to fix this on Github, which is now merged :-)

I just cloned the source and compiled it successfully on Debian, and it all seems to work alright (I can't get hello_world.cc to crash.) If it happens again, could you possibly get me a stack trace?

By the way, if you look at the cookie output code:

    for(Map::Item * Current = Client.Cookies.First; Current; Current = Current->Next)
        if(strcmp(Current->Value, Client.Input.Cookies.Get(Current->Key)))
            Socket << "\r\nSet-Cookie: " << Current->Key << "=" << Current->Value;

The strcmp() line checks the value of the cookie when received from the browser (Client.Input.Cookies) against the value of the cookie with any modifications (Client.Cookies).

Thanks, I changed it to block ".." in any form.

Doesn't this also read right off the end of the string?

No, because if i[1] is 0 (the null terminator), i[1] == '.' will fail.

Duh. Sorry.

No, it will hit the NULL terminator and short circuit.

Reply on news.ycombinator.com