[SSSD] [PATCH] explicit null dereferenced in sss_nss_mc_get_record()

Simo Sorce simo at redhat.com
Thu Jan 3 15:11:44 UTC 2013


On Thu, 2013-01-03 at 09:52 +0100, Pavel Březina wrote:
> https://fedorahosted.org/sssd/ticket/1724
> 
> 
> 
> 
> 
> 
> 
> differences
> between files
> attachment
> (0001-explicit-null-dereferenced-in-sss_nss_mc_get_record.patch)
> 
> From a927c2deeab3b5b54f655e8cbe921e43cc8e1415 Mon Sep 17 00:00:00 2001
> From: =?UTF-8?q?Pavel=20B=C5=99ezina?= <pbrezina at redhat.com>
> Date: Thu, 3 Jan 2013 09:48:39 +0100
> Subject: [PATCH] explicit null dereferenced in sss_nss_mc_get_record()
> 
> https://fedorahosted.org/sssd/ticket/1724
> ---
>  src/sss_client/nss_mc_common.c | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/src/sss_client/nss_mc_common.c
> b/src/sss_client/nss_mc_common.c
> index
> 59f5b257b9c646aa826499623103cd2c45abdbc7..e68e107aa8b2696189cf92c8cee7a1058be54575 100644
> --- a/src/sss_client/nss_mc_common.c
> +++ b/src/sss_client/nss_mc_common.c
> @@ -220,6 +220,11 @@ errno_t sss_nss_mc_get_record(struct
> sss_cli_mc_ctx *ctx,
>              continue;
>          }
>  
> +        if (rec->len == 0 || rec->len == MC_INVALID_VAL32) {
> +            /* record has invalid length */
> +            return EINVAL;
> +        }
> +
>          if (rec_len > buf_size) {
>              free(copy_rec);
>              copy_rec = malloc(rec_len); 

If we have a rec, rec->len cannot be 0 or an invalid value.
But I guess this will mute coverity and give us an extra protection,
however in this case I think we should check for rec->len > max size
rather than just rec->len == MC_INVALID_VAL32. We have an invalid value
for any length that exceeds the mmap region and would get a segfault if
we tried to copy that much data. So if we are going to add this
protection I'd rather do it so we protect for all possible bad values
not just 0 and MC_INVALID_VAL32.

Simo.

-- 
Simo Sorce * Red Hat, Inc * New York




More information about the sssd-devel mailing list