Re: [PATCH 1/1] libselinux: ensure strlen() is not called on NULL

[Stephen Smalley <sds@tycho.nsa.gov>]

On 9/19/19 4:51 PM, Nicolas Iooss wrote:
> On Thu, Sep 19, 2019 at 6:57 PM Stephen Smalley <sds@tycho.nsa.gov> wrote:
>>
>> On 9/18/19 5:04 PM, Nicolas Iooss wrote:
>>> When compile_regex() calls regex_prepare_data() and this function fails
>>> in the following condition:
>>>
>>>       *regex = regex_data_create();
>>>       if (!(*regex))
>>>           return -1;
>>>
>>> ... error_data has been zero-ed and compile_regex() calls:
>>>
>>>       regex_format_error(&error_data,
>>>           regex_error_format_buffer,
>>>           sizeof(regex_error_format_buffer));
>>>
>>> This leads to a call to strlen(error_data->error_buffer), where
>>> error_data->error_buffer is NULL.
>>>
>>> Avoid this by checking that error_data->error_buffer is not NULL before
>>> calling strlen().
>>
>> It seems like regex_format_error() should just return immediately if
>> !error_data->error_code (#ifdef USE_PCRE2) or !error_data->error_buffer
>> (#ifndef USE_PCRE2), since there is no back-end error message to get and
>> report in that situation.
> 
> I agree. I will modify the patch.
> 
> By the way, while reading function regex_format_error() more
> precisely, something seems strange in:
> 
> pos += rc;
> if (pos >= buf_size)
>      goto truncated;
> if (error_data->error_offset > 0) {
>      /* ... */
> }
> pos += rc;
> 
> As rc is not reset to zero, its value is added twice to pos. Is this a
> bug, or am I misunderstanding something?

I think you are correct - it is a bug.

> 
> Thanks,
> Nicolas
>