| [2582] | 1 | From a60a2c6a87331510847de401323bcdf3b9895838 Mon Sep 17 00:00:00 2001 | 
|---|
|  | 2 | From: Adam Glasgall <glasgall@mit.edu> | 
|---|
|  | 3 | Date: Tue, 26 Aug 2014 17:47:45 -0400 | 
|---|
|  | 4 | Subject: [PATCH] Remove r->user != NULL check from ap_process_request_internal | 
|---|
|  | 5 |  | 
|---|
|  | 6 | After the check_user_id hook runs, Apache checks to make sure it's | 
|---|
|  | 7 | identified a user and aborts if this is not the case, to protect the | 
|---|
|  | 8 | auth_checker hook from accidental null pointer | 
|---|
|  | 9 | dereferences. Unfortunately, Scripts's mod_auth_optional relies on | 
|---|
|  | 10 | being able to have r->user still be NULL after check_user_id has run. | 
|---|
|  | 11 |  | 
|---|
|  | 12 | This patch removes the null check. I believe this is safe because | 
|---|
|  | 13 | mod_auth_optional installs its auth_checker hook forcibly at the head | 
|---|
|  | 14 | of the hook chain, and said hook ends authz processing immediately if | 
|---|
|  | 15 | the directory in question has AuthOptional and no default user. | 
|---|
|  | 16 |  | 
|---|
|  | 17 | Signed-off-by: Adam Glasgall <glasgall@mit.edu> | 
|---|
|  | 18 | --- | 
|---|
|  | 19 | server/request.c | 20 -------------------- | 
|---|
|  | 20 | 1 file changed, 20 deletions(-) | 
|---|
|  | 21 |  | 
|---|
|  | 22 | diff --git a/server/request.c b/server/request.c | 
|---|
|  | 23 | index af0a697..9d7e29d 100644 | 
|---|
|  | 24 | --- a/server/request.c | 
|---|
|  | 25 | +++ b/server/request.c | 
|---|
|  | 26 | @@ -244,16 +244,6 @@ AP_DECLARE(int) ap_process_request_internal(request_rec *r) | 
|---|
|  | 27 | if ((access_status = ap_run_check_user_id(r)) != OK) { | 
|---|
|  | 28 | return decl_die(access_status, "check user", r); | 
|---|
|  | 29 | } | 
|---|
|  | 30 | -                if (r->user == NULL) { | 
|---|
|  | 31 | -                    /* don't let buggy authn module crash us in authz */ | 
|---|
|  | 32 | -                    ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r, APLOGNO(00027) | 
|---|
|  | 33 | -                                  "No authentication done but request not " | 
|---|
|  | 34 | -                                  "allowed without authentication for %s. " | 
|---|
|  | 35 | -                                  "Authentication not configured?", | 
|---|
|  | 36 | -                                  r->uri); | 
|---|
|  | 37 | -                    access_status = HTTP_INTERNAL_SERVER_ERROR; | 
|---|
|  | 38 | -                    return decl_die(access_status, "check user", r); | 
|---|
|  | 39 | -                } | 
|---|
|  | 40 | if ((access_status = ap_run_auth_checker(r)) != OK) { | 
|---|
|  | 41 | return decl_die(access_status, "check authorization", r); | 
|---|
|  | 42 | } | 
|---|
|  | 43 | @@ -281,16 +271,6 @@ AP_DECLARE(int) ap_process_request_internal(request_rec *r) | 
|---|
|  | 44 | if ((access_status = ap_run_check_user_id(r)) != OK) { | 
|---|
|  | 45 | return decl_die(access_status, "check user", r); | 
|---|
|  | 46 | } | 
|---|
|  | 47 | -                if (r->user == NULL) { | 
|---|
|  | 48 | -                    /* don't let buggy authn module crash us in authz */ | 
|---|
|  | 49 | -                    ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r, APLOGNO(00028) | 
|---|
|  | 50 | -                                  "No authentication done but request not " | 
|---|
|  | 51 | -                                  "allowed without authentication for %s. " | 
|---|
|  | 52 | -                                  "Authentication not configured?", | 
|---|
|  | 53 | -                                  r->uri); | 
|---|
|  | 54 | -                    access_status = HTTP_INTERNAL_SERVER_ERROR; | 
|---|
|  | 55 | -                    return decl_die(access_status, "check user", r); | 
|---|
|  | 56 | -                } | 
|---|
|  | 57 | if ((access_status = ap_run_auth_checker(r)) != OK) { | 
|---|
|  | 58 | return decl_die(access_status, "check authorization", r); | 
|---|
|  | 59 | } | 
|---|
|  | 60 | -- | 
|---|
|  | 61 | 1.9.1 | 
|---|
|  | 62 |  | 
|---|