Bug 54936 - Some potential bugs reported by static analysis tool canalyze
Summary: Some potential bugs reported by static analysis tool canalyze
Status: NEW
Alias: None
Product: Apache httpd-2
Classification: Unclassified
Component: All (show other bugs)
Version: 2.4.4
Hardware: PC Linux
: P2 normal (vote)
Target Milestone: ---
Assignee: Apache HTTPD Bugs Mailing List
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2013-05-08 10:17 UTC by Zhenbo Xu
Modified: 2014-09-02 09:33 UTC (History)
0 users



Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Zhenbo Xu 2013-05-08 10:17:48 UTC
Hi, 
I'm a developer of a static analysis tool canalyze.
Recently I applied it to httpd-2.4.4
It seems some reports are real after by manually checking:
(I have tried my best to confirming these reports. But I'm not familiar with the source code of httpd. Thus, if there still exist many false alarms, sorry for the trouble. I still hope for your replies!)

1. Use Undefined Value
file: ssl_util.c
function: ssl_util_path_check
At line 124: if (pcm & SSL_PCM_EXISTS && apr_stat(&finfo, path,
pcm & SSL_PCM_EXISTS may be 0, which leaves finfo uninitialized.

At line 129 : if (pcm & SSL_PCM_ISREG && finfo.filetype != APR_REG)
finfo is directly used.

2. Use Undefined Value
file: scoreboard.c
function: ap_find_child_by_pid
At line 394: ap_mpm_query(AP_MPMQ_MAX_DAEMONS, &max_daemons_limit);
may leaves max_daemons_limit uninitialized when ap_run_mpm_query returns -1.

3. Use Undefined Value
file: proxy_util.c
function: proxy_get_host_of_request
At line 401: err = ap_proxy_canon_netloc(r->pool, &url, &user, &password, &host, &port);
host may not be initialized when ap_proxy_canon_netloc returns "Invalid host/port" at line 349.

4. Use Undefined Value
file: dbm.c
function: dav_dbm_open
At line 167: (void) dav_fs_dir_file_name(resource, &dirpath, &fname);
fname may not be initialized when dav_fs_dir_file_name returns at line 285: return dav_new_error(ctx->pool, HTTP_INTERNAL_SERVER_ERROR, 0, rv, ...)

The similar problem occurs at:
file: repos.c
function: dav_fs_copymoveset
line 570: (void) dav_fs_dir_file_name(src, &src_dir, &src_file);
line 571: (void) dav_fs_dir_file_name(dst, &dst_dir, &dst_file);

file: repos.c
function: dav_fs_deleteset
line 623: (void) dav_fs_dir_file_name(resource, &dirpath, &fname);


file: lock.c
function: dav_fs_add_locknull_state
line 936: (void) dav_fs_dir_file_name(resource, &dirpath, &fname);
dirpath is not initialized.

file: lock.c
function: dav_fs_add_locknull_state
line 936: (void) dav_fs_dir_file_name(resource, &dirpath, &fname);

file: lock.c
function: dav_fs_get_locknull_members
line 919: (void) dav_fs_dir_file_name(resource, &dirpath, NULL);

5. Null Pointer
file: lock.c
function: dav_fs_save_lock_record
line 443: (void) dav_dbm_delete(lockdb->info->db, key);
The first argument may be null, which is assigned at

line 434: if ((err = dav_fs_really_open_lockdb(lockdb)) != NULL)
function call dav_fs_really_open_lockdb may assign null to lockdb->info->db at

line 307: err = dav_dbm_open_direct(lockdb->info->pool,..., &lockdb->info->db)
in 

file dbm.c at line 133: *pdb = NULL; 

(Also at line 499: if ((err = dav_dbm_store(lockdb->info->db, key, val)) != NULL))

6. Null Pointer
file: dbm.c
function: dav_propdb_output_value
line 535: apr_datum_t key = dav_build_key(db, name);
key may be zerod at line 317

line 547: dav_append_prop(db->pool, key.dptr, value.dptr, phdr);
Directly uses key.dptr in function dav_append_prop.

7. Null Pointer
file: props.c
function: dav_get_props
line 700: apr_xml_elem *elem = dav_find_child(doc->root, "prop");
Could elem be null?

The similar problem:
file: mod_dav.c
function: dav_cache_badprops
line 1924: elem = dav_find_child(ctx->doc->root, "prop");


8. Null pointer
file: mod_dav.c
function: dav_failed_proppatch
line 2159: if (ctx->err == NULL) 
ctx->err may be null when exiting the for loop

line 2185: ctx->err->status
directly use it.

9. Null Pointer
file: request.c
function: prep_walk_cache
line 347: note = ap_get_request_note(r, t);
note may be null

10. Null pointer
file: props.c
function: dav_prop_validate
line 944: ctx->err = (*priv->provider->patch_validate)(propdb->resource,
priv->provider may be null, which is checked at

line 934: if (!dav_rw_liveprop(propdb, priv))
line 291: if (priv->provider != NULL)

11. Null Pointer
file: config.c
function: socache_shmcb_create
line 294: ctx->data_file = path = ap_server_root_relative(p, arg);
path may be null which is passed to strchr at line 296

12. Memory leak
file: util_ldap_cache.c
function: util_ldap_url_node_copy
line 55: util_ald_free(cache, node->url);
what we should free is node?

13. Use Undefine Value
file: mod_authz_dbm.c
function: dbmfilegroup_check_authorization
line 229: status = get_dbm_grp(r, apr_pstrcat(r->pool, user, ":", realm, NULL),user, conf->grpfile, conf->dbmtype, &groups);
The function call may just return 

14. Use Undefined Value
file: proxy_util.c
function: proxy_get_host_of_request
line 401: err = ap_proxy_canon_netloc(r->pool, &url, &user, &password, &host, &port);
when err is not null, host is not initialized.

15. Use Undefined Value
file: util_expr_eval.c
function: ap_expr_exec_re
line 873: ap_expr_exec_ctx(&ctx)
As ctx.result_string is uninitialized, of which the use at line 809 is invalid.

16. Null Pointer
file: mod_include.c
function: handle_if
line 2318: ap_ssi_get_tag_and_value(ctx, &tag, &expr, SSI_VALUE_RAW);
Could tag be assigned as null?

Also in function handle_elif.

17. Memory leak
file: util_ldap_cache_mgr.c
function: util_ald_create_caches
line 305: if (search_cache && compare_cache && dn_compare_cache)
What if only one or two of caches is null, the other one is not release.
Comment 1 Christophe JAILLET 2013-07-17 05:51:55 UTC
Thanks for the report.

1. Invalid.
In the between of line 124 and 129 you can find:
    AP_DEBUG_ASSERT((pcm & SSL_PCM_EXISTS) ||
                    !(pcm & (SSL_PCM_ISREG|SSL_PCM_ISDIR|SSL_PCM_ISNONZERO)));
So it is expected not to happen.
This was introduced in r1180330 in the 2.4.x branch


2. r1503990


3. r1503991


8. Invalid
At that point ctx->err can not be NULL. It has been set to err424_set or err424_delete which are allocated in the pool.
Should the memory allocation fail, we would abort.


9. Invalid
After the call you mention, we have
     AP_DEBUG_ASSERT(note != NULL);
So it is expected not to be NULL.
Comment 2 Christophe JAILLET 2013-07-17 21:52:38 UTC
12. r1504276


13. Invalid
I think that you mean that 'groups' can be used uninitialized.
If 'get_dbm_grp' just return, the status != APR_SUCCESS
It is checked at line 231.
So we are guaranteed that 'groups' is initialized when it is used.


14. same as 3
Comment 3 Jeff Trawick 2013-08-19 11:46:52 UTC
#12 is now in the 2.4.x branch (r1515372)
Comment 4 Christophe JAILLET 2014-09-02 09:33:02 UTC
#15 is invalid.
The code is guarded by AP_EXPR_FLAG_STRING_RESULT.

See:
    AP_DEBUG_ASSERT((info->flags & AP_EXPR_FLAG_STRING_RESULT) == 0);
in 'ap_expr_exec_re'