On 01/03/2013 04:11 PM, Simo Sorce wrote:
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(a)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.
Hi,
0001:
adds macro for validating record length
This macro is based on sss_mc_is_valid_rec() + checks that length is not
0. I didn't find any min value. If there is any, please tell me.
0002:
fixes the null dereference. I'm not quite sure if it is possible to use
struct sss_cli_mc_ctx->dt_size and data_table to validate the length.
Are those fields identical to those in struct sss_mc_ctx?