Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

DoS - Bug on Range header handling (Trac #184) #90

Closed
edsiper opened this issue Jan 28, 2014 · 4 comments
Closed

DoS - Bug on Range header handling (Trac #184) #90

edsiper opened this issue Jan 28, 2014 · 4 comments

Comments

@edsiper
Copy link
Member

edsiper commented Jan 28, 2014

I've found an issue on the way as Monkey HTTPD handle the Range HTTP header
when receiving Range:bytes=N-N where N is the exact file size, which causes the thread to go into an infinite loop.

mk_http.c:

    /* yyy-xxx */
        if (sh->ranges[0] >= 0 && sh->ranges[1] >= 0) {
            sr->bytes_offset = sh->ranges[0];
            sr->bytes_to_send = labs(sh->ranges[1] - sh->ranges[0]) + 1;
        }

This part calculates the bytes to send, in the case, 1 byte.

int mk_http_send_file(struct client_session *cs, struct session_request *sr)
{
    long int nbytes = 0;

    nbytes = mk_socket_send_file(cs->socket, sr->fd_file,
                                 &sr->bytes_offset, sr->bytes_to_send);

    if (nbytes > 0) {
        sr->bytes_to_send -= nbytes;
        if (sr->bytes_to_send == 0) {
            mk_socket_set_cork_flag(cs->socket, TCP_CORK_OFF);
        }
    }

    sr->loop++;

    if (mk_unlikely(nbytes < 0)) {
        MK_TRACE("sendfile() = -1;");
        return EXIT_ABORT;
    }

    return sr->bytes_to_send;
}

On this function, nbytes is always 0. But the return is always the 1 byte seen above.

mk_request.c

int mk_handler_write(int socket, struct client_session *cs)
...
        if (sr_node->bytes_to_send > 0) {
            /* Request with data to send by static file sender */
            final_status = mk_http_send_file(cs, sr_node);
        }
        else if (sr_node->bytes_to_send < 0) {
            final_status = mk_request_process(cs, sr_node);
        }

        /*
         * If we got an error, we don't want to parse
         * and send information for another pipelined request
         */
        if (final_status > 0) {
            return final_status;

Hence it makes mk_handler_write() return 1, and keep mk_conn_write() trying to
finish the request:

mk_connection.c

int mk_conn_write(int socket)
...
    ret = mk_handler_write(socket, cs);

    /* if ret < 0, means that some error
     * happened in the writer call, in the
     * other hand, 0 means a successful request
     * processed, if ret > 0 means that some data
     * still need to be send.
     */
    if (ret < 0) {
        mk_request_free_list(cs);
        mk_session_remove(socket);
        return -1;
    }
    else if (ret == 0) {
        return mk_http_request_end(socket);
    }
    else if (ret > 0) {
        return 0;
    }

and finally mk_epoll_init perfoming the infinite loop:

mk_epoll.c:

void *mk_epoll_init(int efd, mk_epoll_handlers * handler, int max_events)
...
    while (1) {
        ret = -1;
        num_fds = epoll_wait(efd, events, max_events, MK_EPOLL_WAIT_TIMEOUT);

        for (i = 0; i < num_fds; i++) {
            fd = events[i].data.fd;

            ...
            else if (events[i].events & EPOLLOUT) {
                MK_TRACE("[FD %i] EPoll Event WRITE", fd);
                ret = (*handler->write) (fd);
            }
            ...

            if (ret < 0) {
                MK_TRACE("[FD %i] Epoll Event FORCE CLOSE | ret = %i", fd, ret);
                (*handler->close) (fd);
            }
        }

Migrated from http://bugs.monkey-project.com/ticket/184

{
    "status": "closed", 
    "changetime": "2013-06-20T01:38:51", 
    "description": "I've found an issue on the way as Monkey HTTPD handle the Range HTTP header\nwhen receiving Range:bytes=N-N where N is the exact file size, which causes the thread to go into an infinite loop.\n\nmk_http.c:\n{{{\n\t/* yyy-xxx */\n        if (sh->ranges[0] >= 0 && sh->ranges[1] >= 0) {\n            sr->bytes_offset = sh->ranges[0];\n            sr->bytes_to_send = labs(sh->ranges[1] - sh->ranges[0]) + 1;\n        }\n}}}\n\nThis part calculates the bytes to send, in the case, 1 byte.\n\n{{{\nint mk_http_send_file(struct client_session *cs, struct session_request *sr)\n{\n    long int nbytes = 0;\n\n    nbytes = mk_socket_send_file(cs->socket, sr->fd_file,\n                                 &sr->bytes_offset, sr->bytes_to_send);\n\n    if (nbytes > 0) {\n        sr->bytes_to_send -= nbytes;\n        if (sr->bytes_to_send == 0) {\n            mk_socket_set_cork_flag(cs->socket, TCP_CORK_OFF);\n        }\n    }\n\n    sr->loop++;\n\n    if (mk_unlikely(nbytes < 0)) {\n        MK_TRACE(\"sendfile() = -1;\");\n        return EXIT_ABORT;\n    }\n\n    return sr->bytes_to_send;\n}\n}}}\n\nOn this function, nbytes is always 0. But the return is always the 1 byte seen above.\n\nmk_request.c\n{{{\nint mk_handler_write(int socket, struct client_session *cs)\n...\n        if (sr_node->bytes_to_send > 0) {\n            /* Request with data to send by static file sender */\n            final_status = mk_http_send_file(cs, sr_node);\n        }\n        else if (sr_node->bytes_to_send < 0) {\n            final_status = mk_request_process(cs, sr_node);\n        }\n\n        /*\n         * If we got an error, we don't want to parse\n         * and send information for another pipelined request\n         */\n        if (final_status > 0) {\n            return final_status;\n}}}\n\nHence it makes mk_handler_write() return 1, and keep mk_conn_write() trying to\nfinish the request:\n\nmk_connection.c\n{{{\nint mk_conn_write(int socket)\n...\n    ret = mk_handler_write(socket, cs);\n\n    /* if ret < 0, means that some error\n     * happened in the writer call, in the\n     * other hand, 0 means a successful request\n     * processed, if ret > 0 means that some data\n     * still need to be send.\n     */\n    if (ret < 0) {\n        mk_request_free_list(cs);\n        mk_session_remove(socket);\n        return -1;\n    }\n    else if (ret == 0) {\n        return mk_http_request_end(socket);\n    }\n    else if (ret > 0) {\n        return 0;\n    }\n}}}\n\nand finally mk_epoll_init perfoming the infinite loop:\n\nmk_epoll.c:\n{{{\nvoid *mk_epoll_init(int efd, mk_epoll_handlers * handler, int max_events)\n...\n    while (1) {\n        ret = -1;\n        num_fds = epoll_wait(efd, events, max_events, MK_EPOLL_WAIT_TIMEOUT);\n\n        for (i = 0; i < num_fds; i++) {\n            fd = events[i].data.fd;\n\n            ...\n            else if (events[i].events & EPOLLOUT) {\n                MK_TRACE(\"[FD %i] EPoll Event WRITE\", fd);\n                ret = (*handler->write) (fd);\n            }\n            ...\n\n            if (ret < 0) {\n                MK_TRACE(\"[FD %i] Epoll Event FORCE CLOSE | ret = %i\", fd, ret);\n                (*handler->close) (fd);\n            }\n        }\n}}}", 
    "reporter": "felipensp", 
    "cc": "", 
    "resolution": "fixed", 
    "_ts": "1371692331257976", 
    "component": "HTTP", 
    "summary": "DoS - Bug on Range header handling", 
    "priority": "critical", 
    "keywords": "", 
    "version": "", 
    "time": "2013-06-08T01:10:04", 
    "milestone": "", 
    "owner": "edsiper", 
    "type": "defect"
}
@edsiper
Copy link
Member Author

edsiper commented Jan 28, 2014

Trac update at 20130610T21:37:51: felipensp commented:

CVE-2013-2163 has been assigned to this issue.

@edsiper
Copy link
Member Author

edsiper commented Jan 28, 2014

Trac update at 20130614T17:15:17: felipensp changed component from "Unspecified" to "HTTP"

@edsiper
Copy link
Member Author

edsiper commented Jan 28, 2014

Trac update at 20130617T15:43:36:

  • edsiper changed owner from "" to "edsiper"
  • edsiper changed status from "new" to "assigned"

@edsiper
Copy link
Member Author

edsiper commented Jan 28, 2014

Trac update at 20130620T01:38:51:

  • edsiper commented:

Issue fixed in GIT master through the following commit:

goo.gl/ef3qv


HTTP: ranges fixes and implement 416 status code (fix #90)

The ranges parser did not validate properly the maximum offset
allowed, so if a requester set limit offset equal to file size it
continue processing, internally the sendfile(2) did not failed
returning always zero, this condition was not handled and for hence
that connections keeps running without ending, it could lead to a
DoS.

This patch improves the range headers validation, fix the validation
of mk_http_send_file() routine and when the range is not satisfied, it
will return the status code 416:

  HTTP/1.1 416 Requested Range Not Satisfiable

This fix is being back-ported to v1.2-fixes branch and will be included in Monkey v1.2.2, thanks for your help!

  • edsiper changed resolution from "" to "fixed"
  • edsiper changed status from "assigned" to "closed"

@edsiper edsiper closed this as completed Jan 28, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant