You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@httpd.apache.org by Ruediger Pluem <rp...@apache.org> on 2010/07/20 08:43:05 UTC
Re: svn commit: r965709 - in /httpd/httpd/trunk: CHANGES server/request.c
On 07/20/2010 03:34 AM, niq@apache.org wrote:
> Author: niq
> Date: Tue Jul 20 01:34:39 2010
> New Revision: 965709
>
> URL: http://svn.apache.org/viewvc?rev=965709&view=rev
> Log:
> Don't risk segfault in authz if r->user is not set
> PR 42995
>
> Modified:
> httpd/httpd/trunk/CHANGES
> httpd/httpd/trunk/server/request.c
>
> Modified: httpd/httpd/trunk/CHANGES
> URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/CHANGES?rev=965709&r1=965708&r2=965709&view=diff
> ==============================================================================
> --- httpd/httpd/trunk/CHANGES [utf-8] (original)
> +++ httpd/httpd/trunk/CHANGES [utf-8] Tue Jul 20 01:34:39 2010
> @@ -32,6 +32,10 @@ Changes with Apache 2.3.7
> *) CGI vars: allow PATH to be set by SetEnv, consistent with LD_LIBRARY_PATH
> PR 43906 [Nick Kew]
>
> + *) Core: Extra robustness: don't try authz and segfault if authn
> + fails to set r->user. Log bug and return 500 instead.
> + PR 42995 [Nick Kew]
> +
> Changes with Apache 2.3.6
>
> *) SECURITY: CVE-2009-3555 (cve.mitre.org)
>
> Modified: httpd/httpd/trunk/server/request.c
> URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/server/request.c?rev=965709&r1=965708&r2=965709&view=diff
> ==============================================================================
> --- httpd/httpd/trunk/server/request.c (original)
> +++ httpd/httpd/trunk/server/request.c Tue Jul 20 01:34:39 2010
> @@ -225,6 +225,14 @@ AP_DECLARE(int) ap_process_request_inter
> if ((access_status = ap_run_check_user_id(r)) != OK) {
> return decl_die(access_status, "check user", r);
> }
> + if (r->user == NULL) {
> + /* don't let buggy authn module crash us in authz */
> + ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r,
> + "Buggy authn provider failed to set user for %s",
> + r->uri);
> + access_status = HTTP_INTERNAL_SERVER_ERROR;
> + return decl_die(access_status, "check user", r);
> + }
Is this needed any longer? Shouldn't authz be expected to handle r->user == NULL after Stefans
recent changes?
Regards
Rüdiger
Re: svn commit: r965709 - in /httpd/httpd/trunk: CHANGES server/request.c
Posted by Stefan Fritsch <sf...@sfritsch.de>.
On Tuesday 20 July 2010, Nick Kew wrote:
> On 20 Jul 2010, at 07:43, Ruediger Pluem wrote:
> > Is this needed any longer? Shouldn't authz be expected to handle
> > r->user == NULL after Stefans recent changes?
I have reverted the change that the auth_checker hook is also called
with r->user == NULL. But all 2.3-style authz providers must be able
to handle r->user == NULL.
> Hadn't considered that. An extra check here can't hurt, can it?
You have missed the similar SATISFY_ANY case. ALL and ANY should
behave identical.
Re: svn commit: r965709 - in /httpd/httpd/trunk: CHANGES server/request.c
Posted by Nick Kew <ni...@webthing.com>.
On 20 Jul 2010, at 07:43, Ruediger Pluem wrote:
> Is this needed any longer? Shouldn't authz be expected to handle r->user == NULL after Stefans
> recent changes?
Hadn't considered that. An extra check here can't hurt, can it?
--
Nick Kew