[PATCH 06/14] Cleanup: ensure string is 0 terminated

Jef Driesen jef at libdivecomputer.org
Wed Jan 3 07:23:14 PST 2018


On 29-12-17 01:35, Dirk Hohndel wrote:
> Coverity CID 207790
> 
> Signed-off-by: Dirk Hohndel <dirk at hohndel.org>
> ---
>   src/irda.c | 6 ++++--
>   1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/src/irda.c b/src/irda.c
> index 149808aaa5c2..2964d4d8cb3a 100644
> --- a/src/irda.c
> +++ b/src/irda.c
> @@ -229,10 +229,12 @@ dc_irda_connect_name (dc_iostream_t *abstract, 
> unsigned int address, const char
>   	struct sockaddr_irda peer;
>   	peer.sir_family = AF_IRDA;
>   	peer.sir_addr = address;
> -	if (name)
> +	if (name) {
>   		strncpy (peer.sir_name, name, 25);
> -	else
> +		peer.sir_name[24] = '\0';
> +	} else {
>   		memset (peer.sir_name, 0x00, 25);
> +	}
>   #endif

For this one, I'm not sure whether your fix is the right thing to do. If 
the sir_name field is used as a fixed size byte array (possibly padded 
with zero's) instead of a null terminated string, then null terminating 
it may silently truncate the last byte and cause problems. I don't know 
how the irda stack uses this field, so I simply don't know the right 
answer here.

Checking for possible truncation, and returning an error if the string 
doesn't fit, is probably a safer solution. Even without this patch, 
truncation is already a problem.

PS: Using sizeof instead of hardcoding the length to 25 would be a good 
idea as well.

Jef


More information about the devel mailing list