#3841 The simultaneous use of strncpy and a length-check in client code is confusing Coverity
Opened 21 days ago by jhrozek. Modified 4 days ago

This is a quite minor issue, but we should strive for our code to be easily understandable by Coverity.

In the common.c client-side module in sss_cli_open_socket, we first have this check:

542     if (sizeof(nssaddr.sun_path) <= strlen(socket_name) + 1) {
 543         *errnop = EINVAL;
 544         return -1;
 545     }
 546

But then, we use strncpy and we tell it to write to sizeof(nsaddr.sun_path)

strncpy(nssaddr.sun_path, socket_name, sizeof(nssaddr.sun_path));

This has two issues:
1. strncpy always writes the requested number of bytes, so the rest of the buffer is needlesly zeroed out
2. scanners like Coverity think that, because the strncpy call doesn't itself leave room for NULL-termination, that the resulting buffer can be unterminated. This is not the case, because the check above guards us from using exactly the same sizes of input and the buffer, but it's better to avoid confusing Coverity


Metadata Update from @jhrozek:
- Issue tagged with: easyfix

17 days ago

Metadata Update from @jhrozek:
- Issue set to the milestone: SSSD Patches welcome

4 days ago

Login to comment on this ticket.

Metadata