URL:
https://github.com/SSSD/sssd/pull/601
Title: #601: sbus: integrate sssd with sbus2
jhrozek commented:
"""
There are still some Coverity warnings:
```
Error: CLANG_WARNING:
sssd-1.16.4/src/monitor/monitor.c:2138:9: warning: Value stored to 'ret' is never
read
# ret = notify_startup();
# ^ ~~~~~~~~~~~~~~~~
sssd-1.16.4/src/monitor/monitor.c:2138:9: note: Value stored to 'ret' is never
read
# ret = notify_startup();
# ^ ~~~~~~~~~~~~~~~~
# 2136| * timeout! */
# 2137| if (num_providers == 0 && ctx->services == NULL) {
# 2138|-> ret = notify_startup();
# 2139| }
# 2140|
Error: CPPCHECK_WARNING (CWE-456):
sssd-1.16.4/src/providers/data_provider/dp_target_auth.c:271: error[uninitvar]:
Uninitialized variable: pd
# 269| }
# 270|
# 271|-> if (!should_invoke_selinux(state->provider, pd)) {
# 272| /* State and request related data are freed with sbus_req. */
# 273| dp_pam_reply(state->sbus_req, state->request_name, pd);
Error: UNINIT (CWE-457):
sssd-1.16.4/src/providers/data_provider/dp_target_auth.c:259: var_decl: Declaring variable
"pd" without initializer.
sssd-1.16.4/src/providers/data_provider/dp_target_auth.c:271: uninit_use_in_call: Using
uninitialized value "pd" when calling "should_invoke_selinux".
sssd-1.16.4/src/providers/data_provider/dp_target_auth.c:249:5: read_parm: Reading a
parameter value.
# 269| }
# 270|
# 271|-> if (!should_invoke_selinux(state->provider, pd)) {
# 272| /* State and request related data are freed with sbus_req. */
# 273| dp_pam_reply(state->sbus_req, state->request_name, pd);
Error: UNINIT (CWE-457):
sssd-1.16.4/src/providers/data_provider/dp_target_auth.c:259: var_decl: Declaring variable
"pd" without initializer.
sssd-1.16.4/src/providers/data_provider/dp_target_auth.c:273: uninit_use_in_call: Using
uninitialized value "pd" when calling "dp_pam_reply".
sssd-1.16.4/src/providers/data_provider/dp_target_auth.c:49:5: read_parm: Reading a
parameter value.
# 271| if (!should_invoke_selinux(state->provider, pd)) {
# 272| /* State and request related data are freed with sbus_req. */
# 273|-> dp_pam_reply(state->sbus_req, state->request_name, pd);
# 274| return;
# 275| }
Error: CPPCHECK_WARNING (CWE-456):
sssd-1.16.4/src/providers/data_provider/dp_target_auth.c:306: error[uninitvar]:
Uninitialized variable: pd
# 304|
# 305| /* State and request related data are freed with sbus_req. */
# 306|-> dp_pam_reply(state->sbus_req, state->request_name, pd);
# 307| return;
# 308| }
Error: UNINIT (CWE-457):
sssd-1.16.4/src/providers/data_provider/dp_target_auth.c:293: var_decl: Declaring variable
"pd" without initializer.
sssd-1.16.4/src/providers/data_provider/dp_target_auth.c:306: uninit_use_in_call: Using
uninitialized value "pd" when calling "dp_pam_reply".
sssd-1.16.4/src/providers/data_provider/dp_target_auth.c:49:5: read_parm: Reading a
parameter value.
# 304|
# 305| /* State and request related data are freed with sbus_req. */
# 306|-> dp_pam_reply(state->sbus_req, state->request_name, pd);
# 307| return;
# 308| }
Error: CLANG_WARNING:
sssd-1.16.4/src/responder/ifp/ifp_cache.c:273:5: warning: 2nd function call argument is an
uninitialized value
# iface_ifp_cache_ListByDomain_finish(sbus_req, paths, num_paths);
# ^ ~~~~~
sssd-1.16.4/src/responder/ifp/ifp_cache.c:249:5: note: 'paths' declared without an
initial value
# const char **paths;
# ^~~~~~~~~~~~~~~~~~
sssd-1.16.4/src/responder/ifp/ifp_cache.c:254:9: note: Assuming 'ifp_ctx' is not
equal to NULL
# if (ifp_ctx == NULL) {
# ^~~~~~~~~~~~~~~
sssd-1.16.4/src/responder/ifp/ifp_cache.c:254:5: note: Taking false branch
# if (ifp_ctx == NULL) {
# ^
sssd-1.16.4/src/responder/ifp/ifp_cache.c:260:9: note: Assuming 'domain' is not
equal to NULL
# if (domain == NULL) {
# ^~~~~~~~~~~~~~
sssd-1.16.4/src/responder/ifp/ifp_cache.c:260:5: note: Taking false branch
# if (domain == NULL) {
# ^
sssd-1.16.4/src/responder/ifp/ifp_cache.c:265:11: note: Calling
'ifp_cache_get_cached_objects'
# ret = ifp_cache_get_cached_objects(sbus_req, type, domain,
# ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
sssd-1.16.4/src/responder/ifp/ifp_cache.c:98:25: note: Calling
'ifp_cache_object_class'
# const char *class = ifp_cache_object_class(type);
# ^~~~~~~~~~~~~~~~~~~~~~~~~~~~
sssd-1.16.4/src/responder/ifp/ifp_cache.c:76:5: note: Control jumps to 'case
IFP_CACHE_GROUP:' at line 80
# switch (type) {
# ^
sssd-1.16.4/src/responder/ifp/ifp_cache.c:82:9: note: Execution continues on line 85
# break;
# ^
sssd-1.16.4/src/responder/ifp/ifp_cache.c:98:25: note: Returning from
'ifp_cache_object_class'
# const char *class = ifp_cache_object_class(type);
# ^~~~~~~~~~~~~~~~~~~~~~~~~~~~
sssd-1.16.4/src/responder/ifp/ifp_cache.c:107:9: note: Assuming 'tmp_ctx' is not
equal to NULL
# if (tmp_ctx == NULL) {
# ^~~~~~~~~~~~~~~
sssd-1.16.4/src/responder/ifp/ifp_cache.c:107:5: note: Taking false branch
# if (tmp_ctx == NULL) {
# ^
sssd-1.16.4/src/responder/ifp/ifp_cache.c:111:15: note: Calling
'ifp_cache_build_base_dn'
# base_dn = ifp_cache_build_base_dn(tmp_ctx, type, domain);
# ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
sssd-1.16.4/src/responder/ifp/ifp_cache.c:39:5: note: Control jumps to 'case
IFP_CACHE_GROUP:' at line 43
# switch (type) {
# ^
sssd-1.16.4/src/responder/ifp/ifp_cache.c:45:9: note: Execution continues on line 48
# break;
# ^
sssd-1.16.4/src/responder/ifp/ifp_cache.c:111:15: note: Returning from
'ifp_cache_build_base_dn'
# base_dn = ifp_cache_build_base_dn(tmp_ctx, type, domain);
# ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
sssd-1.16.4/src/responder/ifp/ifp_cache.c:112:9: note: Assuming 'base_dn' is not
equal to NULL
# if (base_dn == NULL) {
# ^~~~~~~~~~~~~~~
sssd-1.16.4/src/responder/ifp/ifp_cache.c:112:5: note: Taking false branch
# if (base_dn == NULL) {
# ^
sssd-1.16.4/src/responder/ifp/ifp_cache.c:122:9: note: Assuming 'ldb_ret' is not
equal to LDB_SUCCESS
# if (ldb_ret != LDB_SUCCESS) {
# ^~~~~~~~~~~~~~~~~~~~~~
sssd-1.16.4/src/responder/ifp/ifp_cache.c:122:5: note: Taking true branch
# if (ldb_ret != LDB_SUCCESS) {
# ^
sssd-1.16.4/src/responder/ifp/ifp_cache.c:123:9: note: Assuming the condition is false
# DEBUG(SSSDBG_CRIT_FAILURE, "Unable to search the cache\n");
# ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
sssd-1.16.4/src/util/debug.h:123:9: note: expanded from macro 'DEBUG'
# if (DEBUG_IS_SET(__debug_macro_level)) { \
# ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
sssd-1.16.4/src/util/debug.h:135:30: note: expanded from macro 'DEBUG_IS_SET'
##define DEBUG_IS_SET(level) (debug_level & (level) || \
# ^~~~~~~~~~~~~~~~~~~~~
sssd-1.16.4/src/responder/ifp/ifp_cache.c:123:9: note: Left side of '||' is
false
sssd-1.16.4/src/util/debug.h:123:9: note: expanded from macro 'DEBUG'
# if (DEBUG_IS_SET(__debug_macro_level)) { \
# ^
sssd-1.16.4/src/util/debug.h:135:30: note: expanded from macro 'DEBUG_IS_SET'
##define DEBUG_IS_SET(level) (debug_level & (level) || \
# ^
sssd-1.16.4/src/responder/ifp/ifp_cache.c:123:9: note: Assuming 'debug_level' is
not equal to DEBUG
# DEBUG(SSSDBG_CRIT_FAILURE, "Unable to search the cache\n");
# ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
sssd-1.16.4/src/util/debug.h:123:9: note: expanded from macro 'DEBUG'
# if (DEBUG_IS_SET(__debug_macro_level)) { \
# ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
sssd-1.16.4/src/util/debug.h:136:30: note: expanded from macro 'DEBUG_IS_SET'
# (debug_level == SSSDBG_UNRESOLVED && \
# ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
sssd-1.16.4/src/responder/ifp/ifp_cache.c:123:9: note: Left side of '&&'
is false
sssd-1.16.4/src/util/debug.h:123:9: note: expanded from macro 'DEBUG'
# if (DEBUG_IS_SET(__debug_macro_level)) { \
# ^
sssd-1.16.4/src/util/debug.h:136:63: note: expanded from macro 'DEBUG_IS_SET'
# (debug_level == SSSDBG_UNRESOLVED && \
# ^
sssd-1.16.4/src/responder/ifp/ifp_cache.c:123:9: note: Loop condition is false. Exiting
loop
sssd-1.16.4/src/util/debug.h:121:35: note: expanded from macro 'DEBUG'
##define DEBUG(level, format, ...) do { \
# ^
sssd-1.16.4/src/responder/ifp/ifp_cache.c:125:9: note: Control jumps to line 148
# goto done;
# ^
sssd-1.16.4/src/responder/ifp/ifp_cache.c:265:11: note: Returning from
'ifp_cache_get_cached_objects'
# ret = ifp_cache_get_cached_objects(sbus_req, type, domain,
# ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
sssd-1.16.4/src/responder/ifp/ifp_cache.c:267:9: note: Assuming 'ret' is equal to
EOK
# if (ret != EOK) {
# ^~~~~~~~~~
sssd-1.16.4/src/responder/ifp/ifp_cache.c:267:5: note: Taking false branch
# if (ret != EOK) {
# ^
sssd-1.16.4/src/responder/ifp/ifp_cache.c:273:5: note: 2nd function call argument is an
uninitialized value
# iface_ifp_cache_ListByDomain_finish(sbus_req, paths, num_paths);
# ^ ~~~~~
# 271| }
# 272|
# 273|-> iface_ifp_cache_ListByDomain_finish(sbus_req, paths, num_paths);
# 274|
# 275| return EOK;
Error: CLANG_WARNING:
sssd-1.16.4/src/sbus/sssd_dbus_connection.c:290:21: warning: 1st function call argument is
an uninitialized value
# conn->address = talloc_strdup(conn, address);
# ^ ~~~~
sssd-1.16.4/src/sbus/sssd_dbus_connection.c:266:5: note: 'conn' declared without
an initial value
# struct sbus_connection *conn;
# ^~~~~~~~~~~~~~~~~~~~~~~~~~~~
sssd-1.16.4/src/sbus/sssd_dbus_connection.c:275:9: note: Assuming 'dbus_conn' is
non-null
# if (!dbus_conn) {
# ^~~~~~~~~~
sssd-1.16.4/src/sbus/sssd_dbus_connection.c:275:5: note: Taking false branch
# if (!dbus_conn) {
# ^
sssd-1.16.4/src/sbus/sssd_dbus_connection.c:283:11: note: Calling
'sbus_init_connection'
# ret = sbus_init_connection(ctx, ev, dbus_conn, SBUS_CONN_TYPE_SHARED,
# ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
sssd-1.16.4/src/sbus/sssd_dbus_connection.c:156:5: note: Assuming the condition is false
# DEBUG(SSSDBG_TRACE_FUNC,"Adding connection %p\n", dbus_conn);
# ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
sssd-1.16.4/src/util/debug.h:123:9: note: expanded from macro 'DEBUG'
# if (DEBUG_IS_SET(__debug_macro_level)) { \
# ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
sssd-1.16.4/src/util/debug.h:135:30: note: expanded from macro 'DEBUG_IS_SET'
##define DEBUG_IS_SET(level) (debug_level & (level) || \
# ^~~~~~~~~~~~~~~~~~~~~
sssd-1.16.4/src/sbus/sssd_dbus_connection.c:156:5: note: Left side of '||' is
false
sssd-1.16.4/src/util/debug.h:123:9: note: expanded from macro 'DEBUG'
# if (DEBUG_IS_SET(__debug_macro_level)) { \
# ^
sssd-1.16.4/src/util/debug.h:135:30: note: expanded from macro 'DEBUG_IS_SET'
##define DEBUG_IS_SET(level) (debug_level & (level) || \
# ^
sssd-1.16.4/src/sbus/sssd_dbus_connection.c:156:5: note: Assuming 'debug_level' is
not equal to DEBUG
# DEBUG(SSSDBG_TRACE_FUNC,"Adding connection %p\n", dbus_conn);
# ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
sssd-1.16.4/src/util/debug.h:123:9: note: expanded from macro 'DEBUG'
# if (DEBUG_IS_SET(__debug_macro_level)) { \
# ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
sssd-1.16.4/src/util/debug.h:136:30: note: expanded from macro 'DEBUG_IS_SET'
# (debug_level == SSSDBG_UNRESOLVED && \
# ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
sssd-1.16.4/src/sbus/sssd_dbus_connection.c:156:5: note: Left side of '&&'
is false
sssd-1.16.4/src/util/debug.h:123:9: note: expanded from macro 'DEBUG'
# if (DEBUG_IS_SET(__debug_macro_level)) { \
# ^
sssd-1.16.4/src/util/debug.h:136:63: note: expanded from macro 'DEBUG_IS_SET'
# (debug_level == SSSDBG_UNRESOLVED && \
# ^
sssd-1.16.4/src/sbus/sssd_dbus_connection.c:156:5: note: Loop condition is false. Exiting
loop
sssd-1.16.4/src/util/debug.h:121:35: note: expanded from macro 'DEBUG'
##define DEBUG(level, format, ...) do { \
# ^
sssd-1.16.4/src/sbus/sssd_dbus_connection.c:167:9: note: Assuming the condition is true
# if (conn->managed_paths == NULL) {
# ^~~~~~~~~~~~~~~~~~~~~~~~~~~
sssd-1.16.4/src/sbus/sssd_dbus_connection.c:167:5: note: Taking true branch
# if (conn->managed_paths == NULL) {
# ^
sssd-1.16.4/src/sbus/sssd_dbus_connection.c:168:9: note: Assuming the condition is false
# DEBUG(SSSDBG_CRIT_FAILURE, "Cannot create object paths hash
table\n");
# ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
sssd-1.16.4/src/util/debug.h:123:9: note: expanded from macro 'DEBUG'
# if (DEBUG_IS_SET(__debug_macro_level)) { \
# ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
sssd-1.16.4/src/util/debug.h:135:30: note: expanded from macro 'DEBUG_IS_SET'
##define DEBUG_IS_SET(level) (debug_level & (level) || \
# ^~~~~~~~~~~~~~~~~~~~~
sssd-1.16.4/src/sbus/sssd_dbus_connection.c:168:9: note: Left side of '||' is
false
sssd-1.16.4/src/util/debug.h:123:9: note: expanded from macro 'DEBUG'
# if (DEBUG_IS_SET(__debug_macro_level)) { \
# ^
sssd-1.16.4/src/util/debug.h:135:30: note: expanded from macro 'DEBUG_IS_SET'
##define DEBUG_IS_SET(level) (debug_level & (level) || \
# ^
sssd-1.16.4/src/sbus/sssd_dbus_connection.c:168:9: note: Assuming 'debug_level' is
not equal to DEBUG
# DEBUG(SSSDBG_CRIT_FAILURE, "Cannot create object paths hash
table\n");
# ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
sssd-1.16.4/src/util/debug.h:123:9: note: expanded from macro 'DEBUG'
# if (DEBUG_IS_SET(__debug_macro_level)) { \
# ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
sssd-1.16.4/src/util/debug.h:136:30: note: expanded from macro 'DEBUG_IS_SET'
# (debug_level == SSSDBG_UNRESOLVED && \
# ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
sssd-1.16.4/src/sbus/sssd_dbus_connection.c:168:9: note: Left side of '&&'
is false
sssd-1.16.4/src/util/debug.h:123:9: note: expanded from macro 'DEBUG'
# if (DEBUG_IS_SET(__debug_macro_level)) { \
# ^
sssd-1.16.4/src/util/debug.h:136:63: note: expanded from macro 'DEBUG_IS_SET'
# (debug_level == SSSDBG_UNRESOLVED && \
# ^
sssd-1.16.4/src/sbus/sssd_dbus_connection.c:168:9: note: Loop condition is false. Exiting
loop
sssd-1.16.4/src/util/debug.h:121:35: note: expanded from macro 'DEBUG'
##define DEBUG(level, format, ...) do { \
# ^
sssd-1.16.4/src/sbus/sssd_dbus_connection.c:283:11: note: Returning from
'sbus_init_connection'
# ret = sbus_init_connection(ctx, ev, dbus_conn, SBUS_CONN_TYPE_SHARED,
# ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
sssd-1.16.4/src/sbus/sssd_dbus_connection.c:285:5: note: Taking true branch
# if (ret != EOK) {
# ^
sssd-1.16.4/src/sbus/sssd_dbus_connection.c:290:21: note: 1st function call argument is an
uninitialized value
# conn->address = talloc_strdup(conn, address);
# ^ ~~~~
# 288|
# 289| /* Store the address for later reconnection */
# 290|-> conn->address = talloc_strdup(conn, address);
# 291|
# 292| dbus_connection_set_exit_on_disconnect(conn->dbus.conn, FALSE);
Error: CLANG_WARNING:
sssd-1.16.4/src/sbus/sssd_dbus_request.c:322:13: warning: Value stored to
'arg_ptr' is never read
# arg_ptr = va_arg(va, void**);
# ^ ~~~~~~~~~~~~~~~~~~
sssd-1.16.4/src/sbus/sssd_dbus_request.c:322:13: note: Value stored to 'arg_ptr'
is never read
# arg_ptr = va_arg(va, void**);
# ^ ~~~~~~~~~~~~~~~~~~
# 320| /* A non array argument */
# 321| } else {
# 322|-> arg_ptr = va_arg(va, void**);
# 323| }
# 324|
Error: TOCTOU (CWE-367):
sssd-1.16.4/src/sbus/sssd_dbus_server.c:141: fs_check_call: Calling function
"readlink" to perform check on "symlink_name".
sssd-1.16.4/src/sbus/sssd_dbus_server.c:171: toctou: Calling function "unlink"
that uses "symlink_name" after a check function. This can cause a time-of-check,
time-of-use race condition.
# 169| }
# 170|
# 171|-> ret = unlink(symlink_name);
# 172| if (ret != 0) {
# 173| ret = errno;
```
"""
See the full comment at
https://github.com/SSSD/sssd/pull/601#issuecomment-412369641