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.
