httpd-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Max Bowsher <ma...@ukf.net>
Subject [PATCH] Re: mod_authz_core:check_provider_list bug?
Date Thu, 09 Mar 2006 11:49:24 GMT
Joe Orton wrote:
> Found by the Coverity report, this one looks like a real bug:
> 
> check_provider_list has a if() branch to handle the passed-in 
> current_provider being NULL, but never sets it to anything else; 
> current_provider is later dereferenced unconditionally.

It looks like the authn-provider implementation was cloned to produce
starting point of the authz-provider implementation, and this code
wasn't removed when it became redundant.

All callers of check_provider_list() ensure that they pass a non-NULL
current_provider.  The AUTHZ_DEFAULT_PROVIDER that is being looked up in
this code is never registered.  The no-providers-configured case is
dealt with in authorize_user(), before check_provider_list() is ever called.

I suggest the following patch:

[[[
* modules/aaa/mod_authz_core.c (check_provider_list):
  Remove redundant code.
* modules/aaa/mod_auth.h (AUTHZ_DEFAULT_PROVIDER):
  Remove redundant definition.
]]]

[[[
Index: modules/aaa/mod_authz_core.c
===================================================================
--- modules/aaa/mod_authz_core.c        (revision 384494)
+++ modules/aaa/mod_authz_core.c        (working copy)
@@ -482,28 +482,10 @@

     const authz_provider *provider;

-    /* For now, if a provider isn't set, we'll be nice and use the file
-     * provider.
-     */
-    if (!current_provider) {
-        provider = ap_lookup_provider(AUTHZ_PROVIDER_GROUP,
-                                      AUTHZ_DEFAULT_PROVIDER, "0");
+    provider = current_provider->provider;
+    apr_table_setn(r->notes, AUTHZ_PROVIDER_NAME_NOTE,
+                   current_provider->provider_name);

-        if (!provider || !provider->check_authorization) {
-            ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r,
-                          "No default authz provider configured");
-            auth_result = AUTHZ_GENERAL_ERROR;
-            return auth_result;
-        }
-        apr_table_setn(r->notes, AUTHZ_PROVIDER_NAME_NOTE,
-                       AUTHZ_DEFAULT_PROVIDER);
-    }
-    else {
-        provider = current_provider->provider;
-        apr_table_setn(r->notes, AUTHZ_PROVIDER_NAME_NOTE,
-                       current_provider->provider_name);
-    }
-
     /* check to make sure that the request method requires
      * authorization before calling the provider
      */
Index: modules/aaa/mod_auth.h
===================================================================
--- modules/aaa/mod_auth.h      (revision 384494)
+++ modules/aaa/mod_auth.h      (working copy)
@@ -38,7 +38,6 @@
 #define AUTHN_PROVIDER_GROUP "authn"
 #define AUTHZ_PROVIDER_GROUP "authz"
 #define AUTHN_DEFAULT_PROVIDER "file"
-#define AUTHZ_DEFAULT_PROVIDER "default"

 #define AUTHZ_GROUP_NOTE "authz_group_note"
 #define AUTHN_PROVIDER_NAME_NOTE "authn_provider_name"
]]]

Max.

Mime
View raw message