Skip to content

Fix inconsistency found by static analyzers #42

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
Anchels opened this issue Feb 11, 2025 · 0 comments
Open

Fix inconsistency found by static analyzers #42

Anchels opened this issue Feb 11, 2025 · 0 comments

Comments

@Anchels
Copy link

Anchels commented Feb 11, 2025

Hello! I was analyzing Nginx modules with the Svace static analyzer. It has found an inconsistency code at the following sections of the code:

static void ngx_http_auth_digest_rbtree_prune_walk(ngx_rbtree_node_t *node,
ngx_rbtree_node_t *sentinel,
time_t now, ngx_log_t *log) {
if (node == sentinel)
return;
if (node->left != sentinel) {
ngx_http_auth_digest_rbtree_prune_walk(node->left, sentinel, now, log);
}
if (node->right != sentinel) {
ngx_http_auth_digest_rbtree_prune_walk(node->right, sentinel, now, log);
}
ngx_http_auth_digest_node_t *dnode = (ngx_http_auth_digest_node_t *)node;
if (dnode->drop_time <= ngx_time()) {
ngx_rbtree_node_t **dropnode =
ngx_array_push(ngx_http_auth_digest_cleanup_list);
dropnode[0] = node;
}
}

and

static void
ngx_http_auth_digest_ev_rbtree_prune_walk(ngx_rbtree_node_t *node,
ngx_rbtree_node_t *sentinel,
time_t now, ngx_log_t *log) {
if (node == sentinel)
return;
if (node->left != sentinel) {
ngx_http_auth_digest_ev_rbtree_prune_walk(node->left, sentinel, now, log);
}
if (node->right != sentinel) {
ngx_http_auth_digest_ev_rbtree_prune_walk(node->right, sentinel, now, log);
}
ngx_http_auth_digest_ev_node_t *dnode =
(ngx_http_auth_digest_ev_node_t *)node;
if (dnode->drop_time <= ngx_time()) {
ngx_rbtree_node_t **dropnode =
ngx_array_push(ngx_http_auth_digest_cleanup_list);
dropnode[0] = node;
}
}


In both methods the result value dropnode of method invocation ngx_array_push is dereferenced without checking for NULL:

and


Here's the source code for function ngx_array_push:

https://github.com/nginx/nginx/blob/ecb809305e54ed15be9f620d56b19ff4e4be7db5/src/core/ngx_array.c#L47-L91

Note that this function can return NULL. Therefore, when using it, it is important to check the result for NULL in order to avoid possible errors. And as a rule, such check is performed in other modules that use this function.

What do you think about adding the NULL checks?


Found by Linux Verification Center (linuxtesting.org) with SVACE.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant