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

remote null pointer dereference trigger in admin handler #1221

Open
mmmds opened this issue Jul 25, 2019 · 2 comments

Comments

@mmmds
Copy link

commented Jul 25, 2019

It's possible to trigger NULL pointer dereference in case if request uses POST method with empty body.

cherokee/handler_admin.c

[...]
204  ret_t
205  cherokee_handler_admin_read_post (cherokee_handler_admin_t *hdl)
206  {
207  	int                      re;
208  	ret_t                    ret;
209  	char                    *tmp;
210  	cherokee_buffer_t        post = CHEROKEE_BUF_INIT;
211  	cherokee_buffer_t        line = CHEROKEE_BUF_INIT;
212  	cherokee_connection_t   *conn = HANDLER_CONN(hdl);
213  
214  	/* Check for the post info
215  	 */
216  	if (! conn->post.has_info) {
217  		conn->error_code = http_bad_request;
218  		return ret_error;
219  	}
220  
221  	/* Process line per line
222  	 */
223  	ret = cherokee_post_read (&conn->post, &conn->socket, &post);
224  	switch (ret) {
225  	case ret_ok:
226  	case ret_eagain:
227  		break;
228  	default:
229  		conn->error_code = http_bad_request;
230  		return ret_error;
231  	}
232  
233  	/* Parse
234  	 */
235  	TRACE (ENTRIES, "Post contains: '%s'\n", post.buf);
236  
237  	cherokee_dwriter_list_open (&hdl->dwriter);
238  
239  	for (tmp = post.buf;;) {
240  		char *end1 = strchr (tmp, CHR_LF);
[...]

If post body is empty then post.buf is NULL and strchr on tmp results in
NULL pointer dereference.

Proof of concept:

curl -d "" http://127.0.0.1:8765/test15/

test15 is the admin handler.

### ASAN report:

=================================================================
==494==ERROR: AddressSanitizer: SEGV on unknown address 0x000000000000 (pc 0x7f221a131ad3 bp 0x7f2213bf4be0 sp 0x7f2213bf4368 T8)
    #0 0x7f221a131ad2  (/lib/x86_64-linux-gnu/libc.so.6+0x89ad2)
    #1 0x7f221b898236 in strchr (/usr/lib/x86_64-linux-gnu/libasan.so.2+0x71236)
    #2 0x5098f2 in cherokee_handler_admin_read_post /home/shm/src/webserver/cherokee/handler_admin.c:240
    #3 0x546dfe in cherokee_handler_read_post /home/shm/src/webserver/cherokee/handler.c:107
    #4 0x53770c in cherokee_connection_read_post /home/shm/src/webserver/cherokee/connection.c:684
    #5 0x489273 in process_active_connections /home/shm/src/webserver/cherokee/thread.c:1235
    #6 0x48df03 in cherokee_thread_step_MULTI_THREAD /home/shm/src/webserver/cherokee/thread.c:2086
    #7 0x48239f in thread_routine /home/shm/src/webserver/cherokee/thread.c:99
    #8 0x7f221ad316b9 in start_thread (/lib/x86_64-linux-gnu/libpthread.so.0+0x76b9)
    #9 0x7f221a1af41c in clone (/lib/x86_64-linux-gnu/libc.so.6+0x10741c)

AddressSanitizer can not provide additional info.
SUMMARY: AddressSanitizer: SEGV ??:0 ??
Thread T8 created by T0 here:
    #0 0x7f221b85d253 in pthread_create (/usr/lib/x86_64-linux-gnu/libasan.so.2+0x36253)
    #1 0x483292 in cherokee_thread_new /home/shm/src/webserver/cherokee/thread.c:247
    #2 0x46ded5 in initialize_server_threads /home/shm/src/webserver/cherokee/server.c:671
    #3 0x4700f2 in cherokee_server_initialize /home/shm/src/webserver/cherokee/server.c:1053
    #4 0x417e26 in common_server_initialization /home/shm/src/webserver/cherokee/main_worker.c:255
    #5 0x41881d in main /home/shm/src/webserver/cherokee/main_worker.c:393
    #6 0x7f221a0c882f in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x2082f)

==494==ABORTING

Setup:

  • Ubuntu 18.04 64 bit
  • source code from github, commit 9a75e65
  • build command:
ac_cv_func_realloc_0_nonnull=yes ac_cv_func_malloc_0_nonnull=yes LDFLAGS="-lasan" LDADD="-lasan" CFLAGS="-fsanitize=address -ggdb -O0 -fprofile-arcs -ftest-coverage" ./configure --prefix=`pwd`/bin --enable-trace --enable-static-module=all --enable-static --enable-shared=no
make
  • files in webroot mkdir /var/www/test{1..20}; for i in seq 1 20; do echo test > test$i/test.html; done
  • configuration file cherokee.txt

found by: Mateusz Kocielski, Michał Dardas from LogicalTrust

@skinkie skinkie self-assigned this Jul 25, 2019

@skinkie skinkie added the t:bug label Jul 25, 2019

@skinkie

This comment has been minimized.

Copy link
Member

commented Jul 25, 2019

Thanks for this very valuable insight. While this only happens when the admin is running, it is obviously bad a practice. When evaluating the code my initial assumption would be that "has_info" should already cover for this, but that only takes care of the headers.

skinkie added a commit that referenced this issue Jul 25, 2019

@skinkie

This comment has been minimized.

Copy link
Member

commented Jul 25, 2019

@mmmds could you validate my pull request?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.