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

Pavel Březina pbrezina at redhat.com
Mon Jan 7 09:43:16 UTC 2013


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 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.
>

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?

-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-memcache-add-macro-that-validates-record-length.patch
Type: text/x-patch
Size: 1636 bytes
Desc: not available
URL: <https://lists.fedorahosted.org/pipermail/sssd-devel/attachments/20130107/fc446d0c/attachment.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0002-explicit-null-dereferenced-in-sss_nss_mc_get_record.patch
Type: text/x-patch
Size: 982 bytes
Desc: not available
URL: <https://lists.fedorahosted.org/pipermail/sssd-devel/attachments/20130107/fc446d0c/attachment-0001.bin>


More information about the sssd-devel mailing list